From 4eecf47ac61724b0e155cb2b3bab3323eb0f8b7b Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Thu, 27 Aug 2026 00:19:34 +0100 Subject: [PATCH] Bound a decimal by the digits it denotes, not just its characters maxNumberOfCharsForNumbers limits the size of numbers read out of a document, but it only checked the length of the lexical value. An exponent is a handful of characters denoting arbitrarily many digits, so the 13 characters of "1E+2000000000" got through and produced a BigDecimal of scale -2000000000: trivial to hold, catastrophic to expand. No option is needed to reach this - Factory.parse does not validate on set, so the lexical check that rejects an exponent never runs. Exponent notation is not part of the xsd:decimal lexical space to begin with, so for conformant input the digit count is bounded by the length of the lexical value and this check never fires. Apply it in parseAsBigDecimal, which is the single point both set_text paths and validateLexical go through. Co-Authored-By: Claude Opus 5 (1M context) --- .../apache/xmlbeans/impl/util/MathUtil.java | 28 +++++++++++++++-- .../misc/checkin/MaxNumberOfCharsTest.java | 31 +++++++++++++++++++ .../xmlbeans/impl/util/TestMathUtil.java | 31 +++++++++++++++++++ 3 files changed, 88 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/apache/xmlbeans/impl/util/MathUtil.java b/src/main/java/org/apache/xmlbeans/impl/util/MathUtil.java index e9e51d91e..497ad1f60 100644 --- a/src/main/java/org/apache/xmlbeans/impl/util/MathUtil.java +++ b/src/main/java/org/apache/xmlbeans/impl/util/MathUtil.java @@ -70,7 +70,8 @@ public static BigDecimal parseAsBigDecimal(String s) { * @param maxNumberOfChars maximum number of characters allowed in the string * @return valid BigDecimal * @throws NumberFormatException if parse fails - * @throws IllegalArgumentException if string is too long + * @throws IllegalArgumentException if the string is too long, or denotes more digits + * than it has characters to spare * @throws NullPointerException if string is null */ public static BigDecimal parseAsBigDecimal(String s, int maxNumberOfChars) { @@ -80,7 +81,30 @@ public static BigDecimal parseAsBigDecimal(String s, int maxNumberOfChars) { if (s.length() > maxNumberOfChars) { throw new IllegalArgumentException("Number has more than " + maxNumberOfChars + " characters"); } - return new BigDecimal(s); + final BigDecimal value = new BigDecimal(s); + final long digits = plainLength(value); + if (digits > maxNumberOfChars) { + throw new IllegalArgumentException("Number denotes " + digits + " digits, more than the " + + maxNumberOfChars + " characters allowed"); + } + return value; + } + + /** + * The number of characters it takes to write the value out without an exponent, which + * is the form xsd:decimal allows and so the form the maximum number of characters is + * meant to bound. Only an exponent can make this exceed the length of the lexical + * value it was parsed from, and an exponent is a handful of characters: the 13 + * characters of "1E+2000000000" denote two billion digits. + */ + private static long plainLength(BigDecimal value) { + final int scale = value.scale(); + final int precision = value.precision(); + return scale <= 0 + // integer digits, ie the significant digits plus the trailing zeros + ? (long) precision - scale + // the digits either side of the decimal point, plus the point itself + : Math.max(precision, (long) scale + 1) + 1; } /** diff --git a/src/test/java/misc/checkin/MaxNumberOfCharsTest.java b/src/test/java/misc/checkin/MaxNumberOfCharsTest.java index 6e771436a..c3c9c87de 100644 --- a/src/test/java/misc/checkin/MaxNumberOfCharsTest.java +++ b/src/test/java/misc/checkin/MaxNumberOfCharsTest.java @@ -183,6 +183,37 @@ public void testIntegralSettersStillRejectOutOfRange() { assertEquals(0L, truncated.getLongValue()); } + @Test + public void testDecimalRejectsExponentDenotingTooManyDigits() throws XmlException { + // "1E+2000000000" is 13 characters, well under any limit, but denotes two billion + // digits. It is not a valid xsd:decimal lexical value in the first place, and the + // limit has to catch it whether or not the value is validated on set. + XmlDecimal value = XmlDecimal.Factory.parse(frag("1E+2000000000")); + assertThrows(XmlValueOutOfRangeException.class, value::getBigDecimalValue); + + XmlOptions exponent = maxChars(XmlOptions.DEFAULT_MAX_NUMBER_CHARS); + exponent.setLoadAllowDecimalExponent(true); + XmlDecimal allowed = XmlDecimal.Factory.parse(frag("1E+2000000000"), exponent); + assertThrows(XmlValueOutOfRangeException.class, allowed::getBigDecimalValue); + + XmlDecimal negative = XmlDecimal.Factory.parse(frag("1E-2000000000"), exponent); + assertThrows(XmlValueOutOfRangeException.class, negative::getBigDecimalValue); + } + + @Test + public void testDecimalStillAcceptsExponentWithinLimit() throws XmlException { + XmlOptions exponent = maxChars(XmlOptions.DEFAULT_MAX_NUMBER_CHARS); + exponent.setLoadAllowDecimalExponent(true); + + XmlDecimal value = XmlDecimal.Factory.parse(frag("1E+20"), exponent); + assertEquals(new BigDecimal("1E+20"), value.getBigDecimalValue()); + + XmlOptions raised = maxChars(4096); + raised.setLoadAllowDecimalExponent(true); + XmlDecimal wide = XmlDecimal.Factory.parse(frag("1E+2000"), raised); + assertEquals(2001, wide.getBigDecimalValue().precision() - wide.getBigDecimalValue().scale()); + } + @Test public void testDecimalHashCodeIgnoresLimit() { // hashCode() must not throw, whatever the limit is, and must stay aligned with diff --git a/src/test/java/org/apache/xmlbeans/impl/util/TestMathUtil.java b/src/test/java/org/apache/xmlbeans/impl/util/TestMathUtil.java index d70666e3a..55bf9b120 100644 --- a/src/test/java/org/apache/xmlbeans/impl/util/TestMathUtil.java +++ b/src/test/java/org/apache/xmlbeans/impl/util/TestMathUtil.java @@ -133,6 +133,37 @@ public void testSafeFloatToIntWithValueOutOfRange() { assertThrows(IllegalArgumentException.class, () -> MathUtil.safeFloatToInt(Float.POSITIVE_INFINITY)); } + @Test + public void testParseAsBigDecimalBoundsDigitsNotJustCharacters() { + // an exponent is a handful of characters denoting arbitrarily many digits, so the + // limit has to apply to what the value denotes, not only to what was written + assertThrows(IllegalArgumentException.class, () -> MathUtil.parseAsBigDecimal("1E+2000000000")); + assertThrows(IllegalArgumentException.class, () -> MathUtil.parseAsBigDecimal("1E-2000000000")); + assertThrows(IllegalArgumentException.class, () -> MathUtil.parseAsBigDecimal("1E+2000")); + assertThrows(IllegalArgumentException.class, () -> MathUtil.parseAsBigDecimal("1E+9", 4)); + + // within the limit an exponent is still accepted, and a raised limit still raises it + assertEquals(new BigDecimal("1E+20"), MathUtil.parseAsBigDecimal("1E+20")); + assertEquals(new BigDecimal("1E+2000"), MathUtil.parseAsBigDecimal("1E+2000", 4096)); + } + + @Test + public void testParseAsBigDecimalAcceptsPlainValues() { + // without an exponent the digit count cannot exceed the length of the lexical + // value, so the new bound never rejects what the length check accepts + assertEquals(new BigDecimal("123.45"), MathUtil.parseAsBigDecimal("123.45")); + assertEquals(new BigDecimal("-0.001"), MathUtil.parseAsBigDecimal("-0.001")); + assertEquals(new BigDecimal("0.10"), MathUtil.parseAsBigDecimal("0.10")); + assertEquals(BigDecimal.ZERO, MathUtil.parseAsBigDecimal("0")); + + StringBuilder sb = new StringBuilder("1"); + for (int i = 1; i < XmlOptions.DEFAULT_MAX_NUMBER_CHARS; i++) { + sb.append('0'); + } + assertEquals(XmlOptions.DEFAULT_MAX_NUMBER_CHARS, + MathUtil.parseAsBigDecimal(sb.toString()).precision()); + } + @Test public void testSafeDoubleToInt() { assertEquals(1, MathUtil.safeDoubleToInt(1.75));