fix(jdbc): prevent BigDecimal DoS in StringUtils.consistentToString#17508
Open
Young-Leo wants to merge 1 commit intoapache:masterfrom
Open
fix(jdbc): prevent BigDecimal DoS in StringUtils.consistentToString#17508Young-Leo wants to merge 1 commit intoapache:masterfrom
Young-Leo wants to merge 1 commit intoapache:masterfrom
Conversation
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A
BigDecimalconstructed from extreme scientific notation (e.g.new BigDecimal("1e1000000000")) hasscale = -1_000_000_000. CallingtoPlainString()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:
which routes through
StringUtils.consistentToString(BigDecimal)→ reflectiveBigDecimal.toPlainString().Reproducer (before the fix)
Observed on JDK 8,
-Xmx512m:1e10001e100000000OutOfMemoryError(swallowed bycatch)1e1000000000(PoC)OutOfMemoryError"1E+1000000000", ~17 msNote: because
InvocationTargetExceptionalso wrapsOutOfMemoryError, the existingcatchblock can silently swallow the OOM on small-heap clients and fall back totoString(). On larger heaps the allocation succeeds and the client process is OOM-killed. Both behaviors are undesirable.Fix
Add a
scaleguard inStringUtils.consistentToString(BigDecimal). When|scale| > 10_000, fall back toBigDecimal.toString()(compact scientific form) instead of invokingtoPlainString(). 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).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.consistentToStringis the only caller ofBigDecimal.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 insetObjectforCHAR/VARCHAR/LONGVARCHAR.Tests
Added
StringUtilsTestcovering:nullinput123.45and1E+2→"100")MAX_PLAIN_STRING_SCALE(still expanded to plain form)1e1000000000(must return compact scientific form, not a 1 GB string)1e-1000000000)All 5 tests pass.
Risk / compatibility
OOM(swallowed) or a multi-GB string and could not have been relied on.Credits
Vulnerability reported by:
Ho1aAs, movonow, zhid889773, WA-888, binghuangczc, errors11, Phoexina, skydiver-jay, V9d0g, FlyFish.