Skip to content

fix(jdbc): prevent BigDecimal DoS in StringUtils.consistentToString#17508

Open
Young-Leo wants to merge 1 commit intoapache:masterfrom
Young-Leo:fix/jdbc-bigdecimal-dos
Open

fix(jdbc): prevent BigDecimal DoS in StringUtils.consistentToString#17508
Young-Leo wants to merge 1 commit intoapache:masterfrom
Young-Leo:fix/jdbc-bigdecimal-dos

Conversation

@Young-Leo
Copy link
Copy Markdown
Contributor

Summary

A BigDecimal constructed from extreme scientific notation (e.g. new BigDecimal("1e1000000000")) has scale = -1_000_000_000. Calling toPlainString() on such a value materializes a plain-decimal string of ~1 × 10⁹ characters (~2 GB), exhausting the JDBC client JVM heap and causing a denial of service.

This path is reachable from user-controlled input via:

IoTDBPreparedStatement.setObject(idx, maliciousBigDecimal, Types.VARCHAR);

which routes through StringUtils.consistentToString(BigDecimal) → reflective BigDecimal.toPlainString().

Reproducer (before the fix)

BigDecimal d = new BigDecimal("1e1000000000");
StringUtils.consistentToString(d); // → OutOfMemoryError

Observed on JDK 8, -Xmx512m:

Input scale Before fix After fix
1e1000 -1000 1001 chars, ~30 ms 1001 chars, ~28 ms (unchanged)
1e100000000 -100 000 000 OutOfMemoryError (swallowed by catch) 12 chars, ~20 ms (scientific notation)
1e1000000000 (PoC) -1 000 000 000 OutOfMemoryError 13 chars "1E+1000000000", ~17 ms

Note: because InvocationTargetException also wraps OutOfMemoryError, the existing catch block can silently swallow the OOM on small-heap clients and fall back to toString(). On larger heaps the allocation succeeds and the client process is OOM-killed. Both behaviors are undesirable.

Fix

Add a scale guard in StringUtils.consistentToString(BigDecimal). When |scale| > 10_000, fall back to BigDecimal.toString() (compact scientific form) instead of invoking toPlainString(). The threshold is orders of magnitude above any realistic decimal precision and is therefore transparent to legitimate data (a 10 000-char plain string is only ~10 KB).

if (decimal.scale() > MAX_PLAIN_STRING_SCALE
    || decimal.scale() < -MAX_PLAIN_STRING_SCALE) {
    return decimal.toString();
}

Also tightens the reflective invocation: toPlainStringMethod.invoke(decimal, (Object[]) null) to remove the varargs-null ambiguity warning.

Why fix it here (and not in IoTDBPreparedStatement)

StringUtils.consistentToString is the only caller of BigDecimal.toPlainString() in the JDBC driver. Fixing it at the root closes every existing and future call site with a single change, rather than duplicating the check in setObject for CHAR/VARCHAR/LONGVARCHAR.

Tests

Added StringUtilsTest covering:

  • null input
  • Normal decimal (123.45 and 1E+2"100")
  • Boundary case at MAX_PLAIN_STRING_SCALE (still expanded to plain form)
  • Regression for the PoC payload 1e1000000000 (must return compact scientific form, not a 1 GB string)
  • Symmetric case for huge positive scale (1e-1000000000)

All 5 tests pass.

Risk / compatibility

  • No API changes.
  • Behavior only differs for pathological values whose plain string would exceed ~10 KB — these inputs previously produced either an OOM (swallowed) or a multi-GB string and could not have been relied on.
  • Normal decimal data is bit-for-bit identical to before.

Credits

Vulnerability reported by:
Ho1aAs, movonow, zhid889773, WA-888, binghuangczc, errors11, Phoexina, skydiver-jay, V9d0g, FlyFish.

A BigDecimal such as new BigDecimal(1e1000000000) has scale -1e9.
Calling toPlainString() on it would materialize a ~1GB String and OOM
the client JVM. This is reachable from IoTDBPreparedStatement.setObject
with targetSqlType in {CHAR, VARCHAR, LONGVARCHAR}, so an attacker that
controls the prepared-statement parameter value can cause a denial of
service on the JDBC client.

Add a scale guard (|scale| <= 10000) before invoking toPlainString. For
any value outside that range, fall back to BigDecimal.toString(), which
is always a few dozen characters. 10000 is vastly larger than any
realistic precision and leaves normal decimal data unaffected.

Also tighten the reflective invocation to pass (Object[]) null to silence
the varargs ambiguity warning.

Adds StringUtilsTest covering null, normal values, the boundary and the
two extreme-scale regressions.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant