Make Object and JsonElement deserialization iterative

Often when Object and JsonElement are deserialized the format of the JSON data is unknown and it might come from an untrusted source. To avoid a StackOverflowError from maliciously crafted JSON, deserialize Object and JsonElement iteratively instead of recursively. Concept based on FasterXML/jackson-databind@51fd2fa But implementation is not based on it.
google · Jun 18, 2021 · 851728d · 851728d
1 parent f319c1b
commit 851728d
Show file tree

Hide file tree

Showing 4 changed files with 258 additions and 50 deletions.
diff --git a/gson/src/main/java/com/google/gson/internal/bind/ObjectTypeAdapter.java b/gson/src/main/java/com/google/gson/internal/bind/ObjectTypeAdapter.java
@@ -27,6 +27,7 @@
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 
@@ -51,42 +52,98 @@ public final class ObjectTypeAdapter extends TypeAdapter<Object> {
     this.gson = gson;
   }
 
-  @Override public Object read(JsonReader in) throws IOException {
-    JsonToken token = in.peek();
-    switch (token) {
-    case BEGIN_ARRAY:
-      List<Object> list = new ArrayList<Object>();
-      in.beginArray();
-      while (in.hasNext()) {
-        list.add(read(in));
-      }
-      in.endArray();
-      return list;
-
-    case BEGIN_OBJECT:
-      Map<String, Object> map = new LinkedTreeMap<String, Object>();
-      in.beginObject();
-      while (in.hasNext()) {
-        map.put(in.nextName(), read(in));
-      }
-      in.endObject();
-      return map;
-
+  /** Read an {@code Object} which cannot have any nested elements */
+  private Object readTerminal(JsonReader in, JsonToken peeked) throws IOException {
+    switch (peeked) {
     case STRING:
       return in.nextString();
-
     case NUMBER:
       return in.nextDouble();
-
     case BOOLEAN:
       return in.nextBoolean();
-
     case NULL:
       in.nextNull();
       return null;
-
     default:
-      throw new IllegalStateException();
+      // When read(JsonReader) is called with JsonReader in invalid state
+      throw new IllegalStateException("Unexpected token: " + peeked);
+    }
+  }
+
+  @Override public Object read(JsonReader in) throws IOException {
+    // Either List or Map
+    Object current;
+
+    JsonToken peeked = in.peek();
+    if (peeked == JsonToken.BEGIN_ARRAY) {
+      current = new ArrayList<Object>();
+      in.beginArray();
+    } else if (peeked == JsonToken.BEGIN_OBJECT) {
+      current = new LinkedTreeMap<String, Object>();
+      in.beginObject();
+    } else {
+      return readTerminal(in, peeked);
+    }
+
+    LinkedList<Object> stack = new LinkedList<Object>();
+
+    while (true) {
+      while (in.hasNext()) {
+        String name = null;
+        // Name is only used for JSON object members
+        if (current instanceof Map) {
+          name = in.nextName();
+        }
+
+        peeked = in.peek();
+        Object value;
+        boolean isNested = false;
+
+        switch (peeked) {
+        case BEGIN_ARRAY:
+          in.beginArray();
+          value = new ArrayList<Object>();
+          isNested = true;
+          break;
+        case BEGIN_OBJECT:
+          in.beginObject();
+          value = new LinkedTreeMap<String, Object>();
+          isNested = true;
+          break;
+        default:
+          value = readTerminal(in, peeked);
+          break;
+        }
+
+        if (current instanceof List) {
+          @SuppressWarnings("unchecked")
+          List<Object> list = (List<Object>) current;
+          list.add(value);
+        } else {
+          @SuppressWarnings("unchecked")
+          Map<String, Object> map = (Map<String, Object>) current;
+          map.put(name, value);
+        }
+
+        if (isNested) {
+          stack.addLast(current);
+          current = value;
+        }
+      }
+
+      // End current element
+      if (current instanceof List) {
+        in.endArray();
+      } else {
+        in.endObject();
+      }
+
+      if (stack.isEmpty()) {
+        return current;
+      } else {
+        // Continue with enclosing element
+        current = stack.removeLast();
+      }
     }
   }
 

diff --git a/gson/src/main/java/com/google/gson/internal/bind/TypeAdapters.java b/gson/src/main/java/com/google/gson/internal/bind/TypeAdapters.java
@@ -31,6 +31,7 @@
 import java.util.Date;
 import java.util.GregorianCalendar;
 import java.util.HashMap;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
@@ -406,7 +407,7 @@ public void write(JsonWriter out, String value) throws IOException {
       out.value(value);
     }
   };
-  
+
   public static final TypeAdapter<BigDecimal> BIG_DECIMAL = new TypeAdapter<BigDecimal>() {
     @Override public BigDecimal read(JsonReader in) throws IOException {
       if (in.peek() == JsonToken.NULL) {
@@ -424,7 +425,7 @@ public void write(JsonWriter out, String value) throws IOException {
       out.value(value);
     }
   };
-  
+
   public static final TypeAdapter<BigInteger> BIG_INTEGER = new TypeAdapter<BigInteger>() {
     @Override public BigInteger read(JsonReader in) throws IOException {
       if (in.peek() == JsonToken.NULL) {
@@ -696,8 +697,9 @@ public void write(JsonWriter out, Locale value) throws IOException {
   public static final TypeAdapterFactory LOCALE_FACTORY = newFactory(Locale.class, LOCALE);
 
   public static final TypeAdapter<JsonElement> JSON_ELEMENT = new TypeAdapter<JsonElement>() {
-    @Override public JsonElement read(JsonReader in) throws IOException {
-      switch (in.peek()) {
+    /** Read a {@link JsonElement} which cannot have any nested elements */
+    private JsonElement readTerminal(JsonReader in, JsonToken peeked) throws IOException {
+      switch (peeked) {
       case STRING:
         return new JsonPrimitive(in.nextString());
       case NUMBER:
@@ -708,28 +710,82 @@ public void write(JsonWriter out, Locale value) throws IOException {
       case NULL:
         in.nextNull();
         return JsonNull.INSTANCE;
-      case BEGIN_ARRAY:
-        JsonArray array = new JsonArray();
+      default:
+        // When read(JsonReader) is called with JsonReader in invalid state
+        throw new IllegalStateException("Unexpected token: " + peeked);
+      }
+    }
+
+    @Override public JsonElement read(JsonReader in) throws IOException {
+      // Either JsonArray or JsonObject
+      JsonElement current;
+
+      JsonToken peeked = in.peek();
+      if (peeked == JsonToken.BEGIN_ARRAY) {
+        current = new JsonArray();
         in.beginArray();
-        while (in.hasNext()) {
-          array.add(read(in));
-        }
-        in.endArray();
-        return array;
-      case BEGIN_OBJECT:
-        JsonObject object = new JsonObject();
+      } else if (peeked == JsonToken.BEGIN_OBJECT) {
+        current = new JsonObject();
         in.beginObject();
+      } else {
+        return readTerminal(in, peeked);
+      }
+
+      LinkedList<JsonElement> stack = new LinkedList<JsonElement>();
+
+      while (true) {
         while (in.hasNext()) {
-          object.add(in.nextName(), read(in));
+          String name = null;
+          // Name is only used for JSON object members
+          if (current instanceof JsonObject) {
+            name = in.nextName();
+          }
+
+          peeked = in.peek();
+          JsonElement value;
+          boolean isNested = false;
+
+          switch (peeked) {
+          case BEGIN_ARRAY:
+            in.beginArray();
+            value = new JsonArray();
+            isNested = true;
+            break;
+          case BEGIN_OBJECT:
+            in.beginObject();
+            value = new JsonObject();
+            isNested = true;
+            break;
+          default:
+            value = readTerminal(in, peeked);
+            break;
+          }
+
+          if (current instanceof JsonArray) {
+            ((JsonArray) current).add(value);
+          } else {
+            ((JsonObject) current).add(name, value);
+          }
+
+          if (isNested) {
+            stack.addLast(current);
+            current = value;
+          }
+        }
+
+        // End current element
+        if (current instanceof JsonArray) {
+          in.endArray();
+        } else {
+          in.endObject();
+        }
+
+        if (stack.isEmpty()) {
+          return current;
+        } else {
+          // Continue with enclosing element
+          current = stack.removeLast();
         }
-        in.endObject();
-        return object;
-      case END_DOCUMENT:
-      case NAME:
-      case END_OBJECT:
-      case END_ARRAY:
-      default:
-        throw new IllegalArgumentException();
       }
     }
 

diff --git a/gson/src/test/java/com/google/gson/JsonParserTest.java b/gson/src/test/java/com/google/gson/JsonParserTest.java
@@ -18,8 +18,8 @@
 
 import java.io.CharArrayReader;
 import java.io.CharArrayWriter;
+import java.io.IOException;
 import java.io.StringReader;
-
 import junit.framework.TestCase;
 
 import com.google.gson.common.TestTypes.BagOfPrimitives;
@@ -90,6 +90,54 @@ public void testParseMixedArray() {
     assertEquals("stringValue", array.get(2).getAsString());
   }
 
+  private static String repeat(String s, int times) {
+    StringBuilder stringBuilder = new StringBuilder(s.length() * times);
+    for (int i = 0; i < times; i++) {
+      stringBuilder.append(s);
+    }
+    return stringBuilder.toString();
+  }
+
+  /** Deeply nested JSON arrays should not cause {@link StackOverflowError} */
+  public void testParseDeeplyNestedArrays() throws IOException {
+    int times = 10000;
+    // [[[ ... ]]]
+    String json = repeat("[", times) + repeat("]", times);
+
+    int actualTimes = 0;
+    JsonArray current = JsonParser.parseString(json).getAsJsonArray();
+    while (true) {
+      actualTimes++;
+      if (current.isEmpty()) {
+        break;
+      }
+      assertEquals(1, current.size());
+      current = current.get(0).getAsJsonArray();
+    }
+    assertEquals(times, actualTimes);
+  }
+
+  /** Deeply nested JSON objects should not cause {@link StackOverflowError} */
+  public void testParseDeeplyNestedObjects() throws IOException {
+    int times = 10000;
+    // {"a":{"a": ... {"a":null} ... }}
+    String json = repeat("{\"a\":", times) + "null" + repeat("}", times);
+
+    int actualTimes = 0;
+    JsonObject current = JsonParser.parseString(json).getAsJsonObject();
+    while (true) {
+      assertEquals(1, current.size());
+      actualTimes++;
+      JsonElement next = current.get("a");
+      if (next.isJsonNull()) {
+        break;
+      } else {
+        current = next.getAsJsonObject();
+      }
+    }
+    assertEquals(times, actualTimes);
+  }
+
   public void testParseReader() {
     StringReader reader = new StringReader("{a:10,b:'c'}");
     JsonElement e = JsonParser.parseReader(reader);