Fixing StackOverflow error

jettison-json · Nov 17, 2022 · 19ae19f · 19ae19f
1 parent 325b51b
commit 19ae19f
Show file tree

Hide file tree

Showing 4 changed files with 114 additions and 9 deletions.
diff --git 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
@@ -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<Object,Object>() :
                 new LinkedHashMap<Object,Object>(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
@@ -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
@@ -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<String,Object> 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<String, Object> map = new HashMap<>();
+            Map<String, Object> 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
+        }
+    }
 }