By default a number literal may not exceed {@code JSONParserBase.MAX_NUMBER_LENGTH} (1000) + * characters. Building a {@link java.math.BigInteger} or {@link java.math.BigDecimal} from a + * decimal string costs superlinear time in the number of digits (measured n^2.0 on Temurin 21), + * so an unbounded number literal lets a single small document consume an arbitrary amount of CPU. + * + *
This option is deliberately opt-OUT rather than opt-in. A limit that has to be switched on + * only protects callers who know it exists: the {@code LIMIT_JSON_DEPTH} fix for CVE-2023-1370 + * had to be switched on, and the consequence was CVE-2024-57699, where callers building their own + * bitmask stayed vulnerable and had to be fixed one by one downstream. Defaulting the limit to ON + * protects hand-built bitmasks too, while leaving an explicit escape hatch for the rare + * application that really does parse arbitrary-precision literals. + * + * @since 2.6.1 + */ + public static final int UNRESTRICTED_NUMBER_LENGTH = 16384; + /** * smart mode, fastest parsing mode. accept lots of non standard JSON syntax ACCEPT_INCOMPLETE * feature is not enabled. in this mode, for backward compatibility * * @since 1.0.6 */ - public static final int MODE_PERMISSIVE = -1 & ~ACCEPT_INCOMPLETE; + public static final int MODE_PERMISSIVE = -1 & ~ACCEPT_INCOMPLETE & ~UNRESTRICTED_NUMBER_LENGTH; /* * smart mode, fastest parsing mode. accept lots of non standard JSON syntax * ACCEPT_INCOMPLETE feature is enabled. * @since 2.6 */ - public static final int MODE_PERMISSIVE_WITH_INCOMPLETE = -1; + public static final int MODE_PERMISSIVE_WITH_INCOMPLETE = -1 & ~UNRESTRICTED_NUMBER_LENGTH; /** * strict RFC4627 mode. diff --git a/json-smart/src/main/java/net/minidev/json/parser/JSONParserBase.java b/json-smart/src/main/java/net/minidev/json/parser/JSONParserBase.java index 76a2274..78256ee 100644 --- a/json-smart/src/main/java/net/minidev/json/parser/JSONParserBase.java +++ b/json-smart/src/main/java/net/minidev/json/parser/JSONParserBase.java @@ -19,6 +19,7 @@ import static net.minidev.json.parser.ParseException.ERROR_UNEXPECTED_EOF; import static net.minidev.json.parser.ParseException.ERROR_UNEXPECTED_JSON_DEPTH; import static net.minidev.json.parser.ParseException.ERROR_UNEXPECTED_LEADING_0; +import static net.minidev.json.parser.ParseException.ERROR_UNEXPECTED_NUMBER_LENGTH; import static net.minidev.json.parser.ParseException.ERROR_UNEXPECTED_TOKEN; import static net.minidev.json.parser.ParseException.ERROR_UNEXPECTED_UNICODE; @@ -41,6 +42,22 @@ abstract class JSONParserBase { /** hard coded maximal depth for JSON parsing */ public static final int MAX_DEPTH = 400; + /** + * hard coded maximal length of a number literal. + * + *
new BigInteger(String, int) and the char[] constructor that new BigDecimal(String) uses + * accumulate their magnitude with the schoolbook destructiveMulAdd loop (OpenJDK 21 + * BigInteger.java lines 559 and 611), so their cost grows superlinearly in the digit count: + * measured n^2.0 on Temurin 21.0.11 over the range 100_000 to 1_600_000 digits. The Schoenhage + * recursive base conversion in BigInteger is used by toString(), not by the String constructor, + * so it does not apply here. An unbounded literal length therefore lets a small document impose + * an arbitrarily large CPU cost on the parsing thread. + * + *
Same default as jackson-core StreamReadConstraints.DEFAULT_MAX_NUM_LEN and org.json + * ParserConfiguration.DEFAULT_MAX_NUMBER_LENGTH. + */ + public static final int MAX_NUMBER_LENGTH = 1000; + protected int depth = 0; JsonReader base; @@ -89,6 +106,7 @@ abstract class JSONParserBase { protected final boolean reject127; protected final boolean unrestictBigDigit; protected final boolean limitJsonDepth; + protected final boolean unrestrictedNumberLength; protected final boolean acceptIncomplete; public JSONParserBase(int permissiveMode) { @@ -107,6 +125,7 @@ public JSONParserBase(int permissiveMode) { this.reject127 = (permissiveMode & JSONParser.REJECT_127_CHAR) > 0; this.unrestictBigDigit = (permissiveMode & JSONParser.BIG_DIGIT_UNRESTRICTED) > 0; this.limitJsonDepth = (permissiveMode & JSONParser.LIMIT_JSON_DEPTH) > 0; + this.unrestrictedNumberLength = (permissiveMode & JSONParser.UNRESTRICTED_NUMBER_LENGTH) > 0; this.acceptIncomplete = (permissiveMode & JSONParser.ACCEPT_INCOMPLETE) > 0; } @@ -160,6 +179,9 @@ protected Number extractFloat() throws ParseException { return asDouble; } } + if (!unrestrictedNumberLength && xs.length() > MAX_NUMBER_LENGTH) { + throw new ParseException(pos, ERROR_UNEXPECTED_NUMBER_LENGTH, xs.length()); + } return new BigDecimal(xs); } @@ -269,6 +291,9 @@ protected Number parseNumber(String s) throws ParseException { max = l; mustCheck = false; } else if (l > max) { + if (!unrestrictedNumberLength && l > MAX_NUMBER_LENGTH) { + throw new ParseException(pos, ERROR_UNEXPECTED_NUMBER_LENGTH, l); + } return new BigInteger(s, 10); } else { max = l - 1; diff --git a/json-smart/src/main/java/net/minidev/json/parser/ParseException.java b/json-smart/src/main/java/net/minidev/json/parser/ParseException.java index 6ff2f49..1b74dd9 100644 --- a/json-smart/src/main/java/net/minidev/json/parser/ParseException.java +++ b/json-smart/src/main/java/net/minidev/json/parser/ParseException.java @@ -31,6 +31,7 @@ public class ParseException extends Exception { public static final int ERROR_UNEXPECTED_DUPLICATE_KEY = 5; public static final int ERROR_UNEXPECTED_LEADING_0 = 6; public static final int ERROR_UNEXPECTED_JSON_DEPTH = 7; + public static final int ERROR_UNEXPECTED_NUMBER_LENGTH = 8; private int errorType; private Object unexpectedObject; @@ -119,6 +120,15 @@ private static String toMessage(int position, int errorType, Object unexpectedOb sb.append(" at position "); sb.append(position); sb.append("."); + } else if (errorType == ERROR_UNEXPECTED_NUMBER_LENGTH) { + // NB: the offending token is deliberately not echoed here, it can be megabytes long. + sb.append("Malicious payload, number literal of "); + sb.append(unexpectedObject); + sb.append(" chars exceeds the maximum of "); + sb.append(JSONParserBase.MAX_NUMBER_LENGTH); + sb.append(", parsing stoped at position "); + sb.append(position); + sb.append("."); } else { sb.append("Unkown error at position "); sb.append(position); diff --git a/json-smart/src/test/java/net/minidev/json/test/TestNumberLengthLimit.java b/json-smart/src/test/java/net/minidev/json/test/TestNumberLengthLimit.java new file mode 100644 index 0000000..28209f9 --- /dev/null +++ b/json-smart/src/test/java/net/minidev/json/test/TestNumberLengthLimit.java @@ -0,0 +1,191 @@ +package net.minidev.json.test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.math.BigDecimal; +import java.math.BigInteger; +import net.minidev.json.JSONObject; +import net.minidev.json.parser.JSONParser; +import net.minidev.json.parser.ParseException; +import org.junit.jupiter.api.Test; + +/** + * A number literal has no length limit, so {@code new BigInteger(String)} / {@code new + * BigDecimal(String)} can be handed an arbitrarily long attacker controlled digit string. Their + * cost grows superlinearly in the digit count: measured n^2.0 on Temurin 21.0.11 over the range + * 100_000 to 1_600_000 digits, where a single 1,600,000 digit literal costs about 32 s of CPU on + * one core. + * + *
These tests pin the limit down the same way {@code TestCVE202457699} pins the depth limit: + * every predefined mode must reject the payload, and ordinary numbers must keep working. + */ +public class TestNumberLengthLimit { + + /** Long enough to be rejected, short enough that an unpatched parser still answers quickly. */ + private static final String LONG_INT = repeat('9', 2000); + + private static final String LONG_DEC = "1." + repeat('3', 2000); + + private static final int[] PREDEFINED_MODES = { + JSONParser.MODE_PERMISSIVE, + JSONParser.MODE_PERMISSIVE_WITH_INCOMPLETE, + JSONParser.MODE_RFC4627, + JSONParser.MODE_JSON_SIMPLE, + JSONParser.MODE_STRICTEST, + }; + + @Test + public void everyPredefinedModeShouldRejectLongIntegerLiteral() { + for (int mode : PREDEFINED_MODES) { + ParseException e = + assertThrows( + ParseException.class, + () -> new JSONParser(mode).parse(LONG_INT), + "mode " + mode + " accepted a " + LONG_INT.length() + " digit integer literal"); + assertEquals(ParseException.ERROR_UNEXPECTED_NUMBER_LENGTH, e.getErrorType()); + } + } + + @Test + public void everyPredefinedModeShouldRejectLongDecimalLiteral() { + for (int mode : PREDEFINED_MODES) { + ParseException e = + assertThrows( + ParseException.class, + () -> new JSONParser(mode).parse(LONG_DEC), + "mode " + mode + " accepted a " + LONG_DEC.length() + " char decimal literal"); + assertEquals(ParseException.ERROR_UNEXPECTED_NUMBER_LENGTH, e.getErrorType()); + } + } + + /** The literal is normally nested in an object, as it would be in a request body. */ + @Test + public void everyPredefinedModeShouldRejectLongLiteralInsideAnObject() { + for (int mode : PREDEFINED_MODES) { + assertThrows( + ParseException.class, + () -> new JSONParser(mode).parse("{\"x\":" + LONG_INT + "}"), + "mode " + mode + " accepted a long integer literal inside an object"); + assertThrows( + ParseException.class, + () -> new JSONParser(mode).parse("[" + LONG_DEC + "]"), + "mode " + mode + " accepted a long decimal literal inside an array"); + } + } + + /** A negative literal takes the other branch of the sign test in parseNumber. */ + @Test + public void negativeLongLiteralShouldBeRejected() { + assertThrows( + ParseException.class, + () -> new JSONParser(JSONParser.MODE_PERMISSIVE).parse("-" + LONG_INT)); + } + + /** Exactly at the limit is still accepted; one over is not. Off by one guard. */ + @Test + public void limitBoundaryIsInclusive() throws Exception { + String atLimit = repeat('9', 1000); + String overLimit = repeat('9', 1001); + + Object ok = new JSONParser(JSONParser.MODE_PERMISSIVE).parse(atLimit); + assertInstanceOf(BigInteger.class, ok); + assertEquals(new BigInteger(atLimit), ok); + + assertThrows( + ParseException.class, () -> new JSONParser(JSONParser.MODE_PERMISSIVE).parse(overLimit)); + } + + /** The limit must not disturb numbers of ordinary size, on any of the numeric branches. */ + @Test + public void ordinaryNumbersAreUnaffected() throws Exception { + JSONParser p = new JSONParser(JSONParser.MODE_PERMISSIVE); + assertEquals(0, p.parse("0")); + assertEquals(-1, p.parse("-1")); + assertEquals(42, p.parse("42")); + assertEquals( + Long.MAX_VALUE, new JSONParser(JSONParser.MODE_PERMISSIVE).parse("9223372036854775807")); + assertEquals( + new BigInteger("9223372036854775808"), + new JSONParser(JSONParser.MODE_PERMISSIVE).parse("9223372036854775808")); + assertEquals(1.5d, new JSONParser(JSONParser.MODE_PERMISSIVE).parse("1.5")); + assertEquals( + new BigDecimal("1.2345678901234567890123"), + new JSONParser(JSONParser.MODE_PERMISSIVE).parse("1.2345678901234567890123")); + + JSONObject o = + (JSONObject) new JSONParser(JSONParser.MODE_PERMISSIVE).parse("{\"a\":1,\"b\":2.5e3}"); + assertEquals(1, o.get("a")); + } + + /** + * The point of making the limit opt-OUT: a caller that hand builds its own bitmask, and so never + * hears about a new flag, is protected anyway. + * + *
This bitmask is not hypothetical. It is the one com.nimbusds:oauth2-oidc-sdk 11.38.2 uses in + * com/nimbusds/oauth2/sdk/util/JSONUtils.java, in both parseJSON and parseJSONKeepingOrder. That + * SDK declares json-smart at compile scope, and it already sets LIMIT_JSON_DEPTH because the + * CVE-2024-57699 fix had to be applied by hand downstream. An opt-in limit would have left it + * exposed a second time. + */ + @Test + public void handBuiltDownstreamBitmaskIsProtectedWithoutChanges() { + int oidcSdkBitmask = + JSONParser.USE_HI_PRECISION_FLOAT + | JSONParser.ACCEPT_TAILLING_SPACE + | JSONParser.LIMIT_JSON_DEPTH; + + assertThrows( + ParseException.class, + () -> new JSONParser(oidcSdkBitmask).parse("{\"x\":" + LONG_INT + "}"), + "hand built downstream bitmask accepted a long integer literal"); + assertThrows( + ParseException.class, + () -> new JSONParser(oidcSdkBitmask).parse("{\"x\":" + LONG_DEC + "}"), + "hand built downstream bitmask accepted a long decimal literal"); + } + + /** + * The escape hatch: an application that genuinely needs arbitrary precision literals opts out + * explicitly. + */ + @Test + public void limitCanBeOptedOutOfExplicitly() throws Exception { + int noLimit = JSONParser.MODE_PERMISSIVE | JSONParser.UNRESTRICTED_NUMBER_LENGTH; + Object r = new JSONParser(noLimit).parse(LONG_INT); + assertInstanceOf(BigInteger.class, r); + assertEquals(new BigInteger(LONG_INT), r); + } + + /** No predefined mode may switch the limit off. This is what CVE-2024-57699 asked for. */ + @Test + public void noPredefinedModeDisablesTheLimit() { + for (int mode : PREDEFINED_MODES) { + assertEquals( + 0, + mode & JSONParser.UNRESTRICTED_NUMBER_LENGTH, + "mode " + mode + " disables the number length limit"); + } + } + + /** The rejection message must not echo the offending token, which can be megabytes long. */ + @Test + public void errorMessageDoesNotEchoTheOffendingToken() { + ParseException e = + assertThrows( + ParseException.class, () -> new JSONParser(JSONParser.MODE_PERMISSIVE).parse(LONG_INT)); + assertTrue( + e.getMessage().length() < 200, "message should not embed the token: " + e.getMessage()); + assertTrue(e.getMessage().contains("2000"), e.getMessage()); + } + + private static String repeat(char c, int n) { + StringBuilder sb = new StringBuilder(n); + for (int i = 0; i < n; i++) { + sb.append(c); + } + return sb.toString(); + } +}