From 19ae19ff57d00dbfa6f6c3af4fc4cb14fb5ca2df Mon Sep 17 00:00:00 2001 From: Colm O hEigeartaigh Date: Wed, 16 Nov 2022 15:36:53 +0000 Subject: [PATCH] Fixing StackOverflow error --- .../org/codehaus/jettison/json/JSONArray.java | 9 ++-- .../codehaus/jettison/json/JSONObject.java | 46 +++++++++++++++-- .../codehaus/jettison/json/JSONTokener.java | 17 +++++-- .../jettison/json/JSONObjectTest.java | 51 +++++++++++++++++++ 4 files changed, 114 insertions(+), 9 deletions(-) diff --git a/src/main/java/org/codehaus/jettison/json/JSONArray.java b/src/main/java/org/codehaus/jettison/json/JSONArray.java index fa5aadb..11acd9a 100644 --- a/src/main/java/org/codehaus/jettison/json/JSONArray.java +++ b/src/main/java/org/codehaus/jettison/json/JSONArray.java @@ -179,8 +179,9 @@ public JSONArray(String string) throws JSONException { /** * Construct a JSONArray from a Collection. * @param collection A Collection. + * @throws JSONException If there is a syntax error. */ - public JSONArray(Collection collection) { + public JSONArray(Collection collection) throws JSONException { this.myArrayList = (collection == null) ? new ArrayList() : new ArrayList(collection); @@ -580,8 +581,9 @@ public JSONArray put(boolean value) { * JSONArray which is produced from a Collection. * @param value A Collection value. * @return this. + * @throws JSONException If there is a syntax error. */ - public JSONArray put(Collection value) { + public JSONArray put(Collection value) throws JSONException { put(new JSONArray(value)); return this; } @@ -631,8 +633,9 @@ public JSONArray put(long value) { * JSONObject which is produced from a Map. * @param value A Map value. * @return this. + * @throws JSONException If there is a syntax error. */ - public JSONArray put(Map value) { + public JSONArray put(Map value) throws JSONException { put(new JSONObject(value)); return this; } diff --git a/src/main/java/org/codehaus/jettison/json/JSONObject.java b/src/main/java/org/codehaus/jettison/json/JSONObject.java index 0c15d74..5932cc9 100644 --- a/src/main/java/org/codehaus/jettison/json/JSONObject.java +++ b/src/main/java/org/codehaus/jettison/json/JSONObject.java @@ -84,6 +84,13 @@ */ public class JSONObject implements Serializable { + /** + * The default recursion depth limit to prevent stack overflow issues on deeply nested structures. + */ + final static int DEFAULT_RECURSION_DEPTH_LIMIT = 500; + + static int RECURSION_DEPTH_LIMIT = DEFAULT_RECURSION_DEPTH_LIMIT; + /** * JSONObject.NULL is equivalent to the value that JavaScript calls null, * whilst Java's null is equivalent to the value that JavaScript calls @@ -257,8 +264,17 @@ public JSONObject(JSONTokener x) throws JSONException { * Construct a JSONObject from a Map. * @param map A map object that can be used to initialize the contents of * the JSONObject. + * @throws JSONException If there is a syntax error. */ - public JSONObject(Map map) { + public JSONObject(Map map) throws JSONException { + this(map, 0); + } + + private JSONObject(Map map, int recursionDepth) throws JSONException { + + if (recursionDepth > RECURSION_DEPTH_LIMIT) { + throw new JSONException("JSONObject has reached recursion depth limit of " + RECURSION_DEPTH_LIMIT); + } this.myHashMap = (map == null) ? new LinkedHashMap() : new LinkedHashMap(map); @@ -268,8 +284,8 @@ public JSONObject(Map map) { if (v instanceof Collection) { myHashMap.put(entry.getKey(), new JSONArray((Collection) v)); } - if (v instanceof Map) { - myHashMap.put(entry.getKey(), new JSONObject((Map) v)); + if (v instanceof Map && v != map) { + myHashMap.put(entry.getKey(), new JSONObject((Map) v, recursionDepth + 1)); } } } @@ -1025,6 +1041,12 @@ public static String quote(String string, boolean escapeForwardSlashAlways) { c = string.charAt(i); switch (c) { case '\\': + // Escape a backslash, but only if it isn't already escaped + if (i == len - 1 || string.charAt(i + 1) != '\\') { + sb.append('\\'); + } + sb.append(c); + break; case '"': sb.append('\\'); sb.append(c); @@ -1319,6 +1341,23 @@ static String valueToString(Object value, int indentFactor, int indent, boolean return quote(value.toString(), escapeForwardSlash); } + /** + * Set the new recursion depth limit to prevent stack overflow issues on deeply nested structures. The default + * value is 500 + * @param newRecursionDepthLimit the new recursion depth limit to set + */ + public void setRecursionDepthLimit(int newRecursionDepthLimit) { + RECURSION_DEPTH_LIMIT = newRecursionDepthLimit; + } + + /** + * Get the new recursion depth limit to prevent stack overflow issues on deeply nested structures. The default + * value is 500 + * @return the recursion depth limit + */ + public int getRecursionDepthLimit() { + return RECURSION_DEPTH_LIMIT; + } /** * Write the contents of the JSONObject as JSON text to a writer. @@ -1396,4 +1435,5 @@ public void setEscapeForwardSlashAlways(boolean escapeForwardSlashAlways) { public Map toMap() { return Collections.unmodifiableMap(myHashMap); } + } diff --git a/src/main/java/org/codehaus/jettison/json/JSONTokener.java b/src/main/java/org/codehaus/jettison/json/JSONTokener.java index 2a1164c..646fa8d 100644 --- a/src/main/java/org/codehaus/jettison/json/JSONTokener.java +++ b/src/main/java/org/codehaus/jettison/json/JSONTokener.java @@ -44,7 +44,9 @@ public class JSONTokener { private int threshold = -1; - + + private int recursionDepth; + /** * Construct a JSONTokener from a string. * @@ -54,7 +56,7 @@ public JSONTokener(String s) { this.myIndex = 0; this.mySource = s.trim(); } - + /** * Construct a JSONTokener from a string. * @@ -423,12 +425,21 @@ public Object nextValue() throws JSONException { } protected JSONObject newJSONObject() throws JSONException { + checkRecursionDepth(); return new JSONObject(this); } - + protected JSONArray newJSONArray() throws JSONException { + checkRecursionDepth(); return new JSONArray(this); } + + private void checkRecursionDepth() throws JSONException { + recursionDepth++; + if (recursionDepth > JSONObject.RECURSION_DEPTH_LIMIT) { + throw new JSONException("JSONTokener has reached recursion depth limit of " + JSONObject.RECURSION_DEPTH_LIMIT); + } + } /** * Skip characters until the next character is the requested character. diff --git a/src/test/java/org/codehaus/jettison/json/JSONObjectTest.java b/src/test/java/org/codehaus/jettison/json/JSONObjectTest.java index acd8246..4125c9d 100644 --- a/src/test/java/org/codehaus/jettison/json/JSONObjectTest.java +++ b/src/test/java/org/codehaus/jettison/json/JSONObjectTest.java @@ -2,7 +2,13 @@ import junit.framework.TestCase; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + public class JSONObjectTest extends TestCase { + public void testEquals() throws Exception { JSONObject aJsonObj = new JSONObject("{\"x\":\"y\"}"); JSONObject bJsonObj = new JSONObject("{\"x\":\"y\"}"); @@ -148,4 +154,49 @@ public void testMalformedArray() throws Exception { } } + // https://github.com/jettison-json/jettison/issues/52 + public void testIssue52() throws Exception { + Map map = new HashMap<>(); + map.put("t",map); + new JSONObject(map); + } + + // https://github.com/jettison-json/jettison/issues/52 + public void testIssue52Recursive() throws Exception { + try { + Map map = new HashMap<>(); + Map map2 = new HashMap<>(); + map.put("t", map2); + map2.put("t", map); + new JSONObject(map); + fail("Failure expected"); + } catch (JSONException e) { + assertTrue(e.getMessage().contains("JSONObject has reached recursion depth limit")); + // expected + } + } + + // https://github.com/jettison-json/jettison/issues/45 + public void testFuzzerTestCase() throws Exception, JSONException { + try { + new JSONObject("{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{{\"G\":[30018084,1,6,32768,1,1,6,1,1]}:[3,1,6,32768,1,1,6,1,1]}:[3,1,6,32768,1,1,6,1,1]}:[3,1,6,32768,1,1,6,1,1]}:[3,1,6,32768,1,1,6,1,1]}:[3,1,6,32768,1,1,6,1,1]}:[3,1,6,32768,1,1,6,1,1]}:[3,1,6,38,1,1,6,1,1]}:[3,1,6,32768,1,1,6,1,1]}:[3,1,6,32768,1,1,6,1,1]}:[3,1,6,32768,1,1,6,1,1]}:[3,1,6,32768,1,1,6,1,1]}:[3,1,6,32768,1,1,6,1,1]}:[3,1,6,32768,1,1,6,1,0]}:[3,1,6,32768,1,1,6,1,1]}:[3,1,6,32768,1,340282366920938463463374607431768211458,6,1,1]}:[32768,1,1,6,1,0]}:[3,1,6,32768,1,1,6,1,1]}:[3,1,6,32768,1,340282366920938463463374607431768211458,6,1,1]}:[3,1,6,32768,1,1,6,1,1]}:[3,1,6,32768,9 68,1,127,1,1]}:[3,1,6,32768,1,1,6,1,1]}:[3,1,6,32768,1,1,6,1,1]}:[3,1,6,32768,1,1,6,1,1]}:[3,1,6,32768,1,1,6,1,1]}:[3,1,6,32768,1,1,6,1,1]}:[3,1,6,32768,1,1,6,1,9223372036854775807]}:[3,1,6,32768,1,1,6,1,1]}:[3,1,10,32768,1,1,6,1,1]}"); + fail("Failure expected"); + } catch (JSONException ex) { + // expected + } + } + + public void testFuzzerTestCase2() throws Exception { + StringBuilder sb = new StringBuilder(); + for (int i = 0; i < 100000; i++) { + sb.append("{"); + } + try { + new JSONObject(sb.toString()); + fail("Failure expected"); + } catch (JSONException e) { + assertTrue(e.getMessage().contains("JSONTokener has reached recursion depth limit")); + // expected + } + } }