From bf3a2ff1025f9e6d7d9b2b7e15d40ef10d7e664c Mon Sep 17 00:00:00 2001 From: Claire Gordon Date: Tue, 26 Mar 2024 10:58:11 -0400 Subject: [PATCH 1/2] Disallow nested objects and arrays as keys in objects Port of https://github.com/stleary/JSON-java/pull/772 to partially remediate https://www.cve.org/CVERecord?id=CVE-2023-5072 , where nested keys can allow relatively small inputs to cause OOM errors through recursion. Test by: - package & import into alpha locally - confirm a suite of unit tests depending on JSONObjects passes - verify that the following CVE Proof-of-concept fails with an 'unexpected character' exception: https://security.snyk.io/vuln/SNYK-JAVA-ORGJSON-5962464 --- org/json/JSONObject.java | 3 +-- org/json/JSONTokener.java | 15 +++++++++++---- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/org/json/JSONObject.java b/org/json/JSONObject.java index 63fd51e..ee5f968 100644 --- a/org/json/JSONObject.java +++ b/org/json/JSONObject.java @@ -187,8 +187,7 @@ public JSONObject(JSONTokener x) throws JSONException { case '}': return; default: - x.back(); - key = x.nextValue().toString(); + key = x.nextSimpleValue(c).toString(); } /* diff --git a/org/json/JSONTokener.java b/org/json/JSONTokener.java index 784ae45..c5d0715 100644 --- a/org/json/JSONTokener.java +++ b/org/json/JSONTokener.java @@ -363,12 +363,8 @@ public String nextTo(String delimiters) throws JSONException { */ public Object nextValue() throws JSONException { char c = nextClean(); - String s; switch (c) { - case '"': - case '\'': - return nextString(c); case '{': back(); return new JSONObject(this); @@ -377,6 +373,17 @@ public Object nextValue() throws JSONException { back(); return new JSONArray(this); } + return nextSimpleValue(c); + } + + Object nextSimpleValue(char c) throws JSONException { + String s; + + switch (c) { + case '"': + case '\'': + return this.nextString(c); + } /* * Handle unquoted text. This could be the values true, false, or From 4fa27f1bf2972276475bfac2edd27f083209d1b0 Mon Sep 17 00:00:00 2001 From: Claire Gordon Date: Tue, 26 Mar 2024 13:32:49 -0400 Subject: [PATCH 2/2] JSON parsing should detect embedded \0 values See: https://github.com/stleary/JSON-java/issues/758 https://github.com/stleary/JSON-java/pull/759 Port pull #759 from stleary/JSON-java to help address OOM errors described in https://www.cve.org/CVERecord?id=CVE-2023-5072 To support the JSONTokener.end() function this relies on, port over the 'eof' flag & set in all locations it's used in the latest JSON-java. Use the String next(int n) implementation from more recent java versions so we can properly check end() while reading a group of characters. Test by: - importing into alpha locally & running all tests that depend on //thirdparty:json - verifying that Snyk's proof-of-concept does not cause OOMs: https://security.snyk.io/vuln/SNYK-JAVA-ORGJSON-5962464 --- org/json/JSONObject.java | 3 +++ org/json/JSONTokener.java | 40 ++++++++++++++++++--------------------- 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/org/json/JSONObject.java b/org/json/JSONObject.java index ee5f968..b0682c8 100644 --- a/org/json/JSONObject.java +++ b/org/json/JSONObject.java @@ -214,6 +214,9 @@ public JSONObject(JSONTokener x) throws JSONException { if (x.nextClean() == '}') { return; } + if (x.end()) { + throw x.syntaxError("A JSONObject text must end with '}'"); + } x.back(); break; case '}': diff --git a/org/json/JSONTokener.java b/org/json/JSONTokener.java index c5d0715..7580cd1 100644 --- a/org/json/JSONTokener.java +++ b/org/json/JSONTokener.java @@ -39,6 +39,7 @@ of this software and associated documentation files (the "Software"), to deal public class JSONTokener { private int index; + private boolean eof; private Reader reader; private char lastChar; private boolean useLastChar; @@ -50,6 +51,7 @@ public class JSONTokener { * @param reader A reader. */ public JSONTokener(Reader reader) { + this.eof = false; this.reader = reader.markSupported() ? reader : new BufferedReader(reader); this.useLastChar = false; @@ -78,6 +80,7 @@ public void back() throws JSONException { } index -= 1; useLastChar = true; + this.eof = false; } @@ -101,6 +104,10 @@ public static int dehexchar(char c) { return -1; } + public boolean end() { + return eof && !useLastChar; + } + /** * Determine if the source string still contains characters that next() @@ -108,8 +115,8 @@ public static int dehexchar(char c) { * @return true if not yet at the end of the source. */ public boolean more() throws JSONException { - char nextChar = next(); - if (nextChar == 0) { + next(); + if (end()) { return false; } back(); @@ -138,6 +145,7 @@ public char next() throws JSONException { } if (c <= 0) { // End of stream + this.eof = true; this.lastChar = 0; return 0; } @@ -181,27 +189,13 @@ public String next(int n) throws JSONException { char[] buffer = new char[n]; int pos = 0; - if (this.useLastChar) { - this.useLastChar = false; - buffer[0] = this.lastChar; - pos = 1; - } - - try { - int len; - while ((pos < n) && ((len = reader.read(buffer, pos, n - pos)) != -1)) { - pos += len; + while (pos < n) { + buffer[pos] = this.next(); + if (this.end()) { + throw this.syntaxError("Substring bounds error"); } - } catch (IOException exc) { - throw new JSONException(exc); + pos += 1; } - this.index += pos; - - if (pos < n) { - throw syntaxError("Substring bounds error"); - } - - this.lastChar = buffer[n - 1]; return new String(buffer); } @@ -400,7 +394,9 @@ Object nextSimpleValue(char c) throws JSONException { sb.append(c); c = next(); } - back(); + if (!this.eof) { + back(); + } /* * If it is true, false, or null, return the proper value.