Skip to content

Commit 539952e

Browse files
Marcono1234Marcono1234
committed
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.
1 parent f319c1b commit 539952e

File tree

6 files changed

+338
-48
lines changed

6 files changed

+338
-48
lines changed

‎gson/src/main/java/com/google/gson/internal/bind/ObjectTypeAdapter.java

Lines changed: 79 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727

2828
import java.io.IOException;
2929
import java.util.ArrayList;
30+
import java.util.LinkedList;
3031
import java.util.List;
3132
import java.util.Map;
3233

@@ -51,42 +52,97 @@ public final class ObjectTypeAdapter extends TypeAdapter<Object> {
5152
this.gson = gson;
5253
}
5354

54-
@Override public Object read(JsonReader in) throws IOException {
55-
JsonToken token = in.peek();
56-
switch (token) {
57-
case BEGIN_ARRAY:
58-
List<Object> list = new ArrayList<Object>();
55+
/**
56+
* Tries to begin reading a JSON array or JSON object, returning {@code null} if
57+
* the next element is neither of those.
58+
*/
59+
private Object tryBeginNesting(JsonReader in, JsonToken peeked) throws IOException {
60+
if (peeked == JsonToken.BEGIN_ARRAY) {
5961
in.beginArray();
60-
while (in.hasNext()) {
61-
list.add(read(in));
62-
}
63-
in.endArray();
64-
return list;
65-
66-
case BEGIN_OBJECT:
67-
Map<String, Object> map = new LinkedTreeMap<String, Object>();
62+
return new ArrayList<Object>();
63+
} else if (peeked == JsonToken.BEGIN_OBJECT) {
6864
in.beginObject();
69-
while (in.hasNext()) {
70-
map.put(in.nextName(), read(in));
71-
}
72-
in.endObject();
73-
return map;
65+
return new LinkedTreeMap<String, Object>();
66+
} else {
67+
return null;
68+
}
69+
}
7470

71+
/** Reads an {@code Object} which cannot have any nested elements */
72+
private Object readTerminal(JsonReader in, JsonToken peeked) throws IOException {
73+
switch (peeked) {
7574
case STRING:
7675
return in.nextString();
77-
7876
case NUMBER:
7977
return in.nextDouble();
80-
8178
case BOOLEAN:
8279
return in.nextBoolean();
83-
8480
case NULL:
8581
in.nextNull();
8682
return null;
87-
8883
default:
89-
throw new IllegalStateException();
84+
// When read(JsonReader) is called with JsonReader in invalid state
85+
throw new IllegalStateException("Unexpected token: " + peeked);
86+
}
87+
}
88+
89+
@Override public Object read(JsonReader in) throws IOException {
90+
// Either List or Map
91+
Object current;
92+
JsonToken peeked = in.peek();
93+
94+
current = tryBeginNesting(in, peeked);
95+
if (current == null) {
96+
return readTerminal(in, peeked);
97+
}
98+
99+
LinkedList<Object> stack = new LinkedList<Object>();
100+
101+
while (true) {
102+
while (in.hasNext()) {
103+
String name = null;
104+
// Name is only used for JSON object members
105+
if (current instanceof Map) {
106+
name = in.nextName();
107+
}
108+
109+
peeked = in.peek();
110+
Object value = tryBeginNesting(in, peeked);
111+
boolean isNesting = value != null;
112+
113+
if (value == null) {
114+
value = readTerminal(in, peeked);
115+
}
116+
117+
if (current instanceof List) {
118+
@SuppressWarnings("unchecked")
119+
List<Object> list = (List<Object>) current;
120+
list.add(value);
121+
} else {
122+
@SuppressWarnings("unchecked")
123+
Map<String, Object> map = (Map<String, Object>) current;
124+
map.put(name, value);
125+
}
126+
127+
if (isNesting) {
128+
stack.addLast(current);
129+
current = value;
130+
}
131+
}
132+
133+
// End current element
134+
if (current instanceof List) {
135+
in.endArray();
136+
} else {
137+
in.endObject();
138+
}
139+
140+
if (stack.isEmpty()) {
141+
return current;
142+
} else {
143+
// Continue with enclosing element
144+
current = stack.removeLast();
145+
}
90146
}
91147
}
92148

‎gson/src/main/java/com/google/gson/internal/bind/TypeAdapters.java

Lines changed: 78 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import java.util.Date;
3232
import java.util.GregorianCalendar;
3333
import java.util.HashMap;
34+
import java.util.LinkedList;
3435
import java.util.List;
3536
import java.util.Locale;
3637
import java.util.Map;
@@ -406,7 +407,7 @@ public void write(JsonWriter out, String value) throws IOException {
406407
out.value(value);
407408
}
408409
};
409-
410+
410411
public static final TypeAdapter<BigDecimal> BIG_DECIMAL = new TypeAdapter<BigDecimal>() {
411412
@Override public BigDecimal read(JsonReader in) throws IOException {
412413
if (in.peek() == JsonToken.NULL) {
@@ -424,7 +425,7 @@ public void write(JsonWriter out, String value) throws IOException {
424425
out.value(value);
425426
}
426427
};
427-
428+
428429
public static final TypeAdapter<BigInteger> BIG_INTEGER = new TypeAdapter<BigInteger>() {
429430
@Override public BigInteger read(JsonReader in) throws IOException {
430431
if (in.peek() == JsonToken.NULL) {
@@ -696,8 +697,25 @@ public void write(JsonWriter out, Locale value) throws IOException {
696697
public static final TypeAdapterFactory LOCALE_FACTORY = newFactory(Locale.class, LOCALE);
697698

698699
public static final TypeAdapter<JsonElement> JSON_ELEMENT = new TypeAdapter<JsonElement>() {
699-
@Override public JsonElement read(JsonReader in) throws IOException {
700-
switch (in.peek()) {
700+
/**
701+
* Tries to begin reading a JSON array or JSON object, returning {@code null} if
702+
* the next element is neither of those.
703+
*/
704+
private JsonElement tryBeginNesting(JsonReader in, JsonToken peeked) throws IOException {
705+
if (peeked == JsonToken.BEGIN_ARRAY) {
706+
in.beginArray();
707+
return new JsonArray();
708+
} else if (peeked == JsonToken.BEGIN_OBJECT) {
709+
in.beginObject();
710+
return new JsonObject();
711+
} else {
712+
return null;
713+
}
714+
}
715+
716+
/** Reads a {@link JsonElement} which cannot have any nested elements */
717+
private JsonElement readTerminal(JsonReader in, JsonToken peeked) throws IOException {
718+
switch (peeked) {
701719
case STRING:
702720
return new JsonPrimitive(in.nextString());
703721
case NUMBER:
@@ -708,28 +726,65 @@ public void write(JsonWriter out, Locale value) throws IOException {
708726
case NULL:
709727
in.nextNull();
710728
return JsonNull.INSTANCE;
711-
case BEGIN_ARRAY:
712-
JsonArray array = new JsonArray();
713-
in.beginArray();
729+
default:
730+
// When read(JsonReader) is called with JsonReader in invalid state
731+
throw new IllegalStateException("Unexpected token: " + peeked);
732+
}
733+
}
734+
735+
@Override public JsonElement read(JsonReader in) throws IOException {
736+
// Either JsonArray or JsonObject
737+
JsonElement current;
738+
JsonToken peeked = in.peek();
739+
740+
current = tryBeginNesting(in, peeked);
741+
if (current == null) {
742+
return readTerminal(in, peeked);
743+
}
744+
745+
LinkedList<JsonElement> stack = new LinkedList<JsonElement>();
746+
747+
while (true) {
714748
while (in.hasNext()) {
715-
array.add(read(in));
749+
String name = null;
750+
// Name is only used for JSON object members
751+
if (current instanceof JsonObject) {
752+
name = in.nextName();
753+
}
754+
755+
peeked = in.peek();
756+
JsonElement value = tryBeginNesting(in, peeked);
757+
boolean isNesting = value != null;
758+
759+
if (value == null) {
760+
value = readTerminal(in, peeked);
761+
}
762+
763+
if (current instanceof JsonArray) {
764+
((JsonArray) current).add(value);
765+
} else {
766+
((JsonObject) current).add(name, value);
767+
}
768+
769+
if (isNesting) {
770+
stack.addLast(current);
771+
current = value;
772+
}
716773
}
717-
in.endArray();
718-
return array;
719-
case BEGIN_OBJECT:
720-
JsonObject object = new JsonObject();
721-
in.beginObject();
722-
while (in.hasNext()) {
723-
object.add(in.nextName(), read(in));
774+
775+
// End current element
776+
if (current instanceof JsonArray) {
777+
in.endArray();
778+
} else {
779+
in.endObject();
780+
}
781+
782+
if (stack.isEmpty()) {
783+
return current;
784+
} else {
785+
// Continue with enclosing element
786+
current = stack.removeLast();
724787
}
725-
in.endObject();
726-
return object;
727-
case END_DOCUMENT:
728-
case NAME:
729-
case END_OBJECT:
730-
case END_ARRAY:
731-
default:
732-
throw new IllegalArgumentException();
733788
}
734789
}
735790

‎gson/src/test/java/com/google/gson/JsonParserParameterizedTest.java

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
package com.google.gson;
2+
3+
import static org.junit.Assert.assertEquals;
4+
5+
import java.io.IOException;
6+
import java.util.Arrays;
7+
8+
import org.junit.Test;
9+
import org.junit.runner.RunWith;
10+
import org.junit.runners.Parameterized;
11+
import org.junit.runners.Parameterized.Parameter;
12+
import org.junit.runners.Parameterized.Parameters;
13+
14+
@RunWith(Parameterized.class)
15+
public class JsonParserParameterizedTest {
16+
@Parameters
17+
public static Iterable<String> data() {
18+
return Arrays.asList(
19+
"[]",
20+
"{}",
21+
"null",
22+
"1.0",
23+
"true",
24+
"\"string\"",
25+
"[true,1.0,null,{},2.0,{\"a\":[false]},[3.0,\"test\"],4.0]",
26+
"{\"\":1.0,\"a\":true,\"b\":null,\"c\":[],\"d\":{\"a1\":2.0,\"b2\":[true,{\"a3\":3.0}]},\"e\":[{\"f\":4.0},\"test\"]}"
27+
);
28+
}
29+
30+
private final TypeAdapter<JsonElement> adapter = new Gson().getAdapter(JsonElement.class);
31+
@Parameter
32+
public String json;
33+
34+
@Test
35+
public void testParse() throws IOException {
36+
JsonElement deserialized = JsonParser.parseString(json);
37+
String actualSerialized = adapter.toJson(deserialized);
38+
39+
// Serialized JsonElement should be the same as original JSON
40+
assertEquals(json, actualSerialized);
41+
}
42+
}

‎gson/src/test/java/com/google/gson/JsonParserTest.java

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@
1818

1919
import java.io.CharArrayReader;
2020
import java.io.CharArrayWriter;
21+
import java.io.IOException;
2122
import java.io.StringReader;
22-
2323
import junit.framework.TestCase;
2424

2525
import com.google.gson.common.TestTypes.BagOfPrimitives;
@@ -90,6 +90,54 @@ public void testParseMixedArray() {
9090
assertEquals("stringValue", array.get(2).getAsString());
9191
}
9292

93+
private static String repeat(String s, int times) {
94+
StringBuilder stringBuilder = new StringBuilder(s.length() * times);
95+
for (int i = 0; i < times; i++) {
96+
stringBuilder.append(s);
97+
}
98+
return stringBuilder.toString();
99+
}
100+
101+
/** Deeply nested JSON arrays should not cause {@link StackOverflowError} */
102+
public void testParseDeeplyNestedArrays() throws IOException {
103+
int times = 10000;
104+
// [[[ ... ]]]
105+
String json = repeat("[", times) + repeat("]", times);
106+
107+
int actualTimes = 0;
108+
JsonArray current = JsonParser.parseString(json).getAsJsonArray();
109+
while (true) {
110+
actualTimes++;
111+
if (current.isEmpty()) {
112+
break;
113+
}
114+
assertEquals(1, current.size());
115+
current = current.get(0).getAsJsonArray();
116+
}
117+
assertEquals(times, actualTimes);
118+
}
119+
120+
/** Deeply nested JSON objects should not cause {@link StackOverflowError} */
121+
public void testParseDeeplyNestedObjects() throws IOException {
122+
int times = 10000;
123+
// {"a":{"a": ... {"a":null} ... }}
124+
String json = repeat("{\"a\":", times) + "null" + repeat("}", times);
125+
126+
int actualTimes = 0;
127+
JsonObject current = JsonParser.parseString(json).getAsJsonObject();
128+
while (true) {
129+
assertEquals(1, current.size());
130+
actualTimes++;
131+
JsonElement next = current.get("a");
132+
if (next.isJsonNull()) {
133+
break;
134+
} else {
135+
current = next.getAsJsonObject();
136+
}
137+
}
138+
assertEquals(times, actualTimes);
139+
}
140+
93141
public void testParseReader() {
94142
StringReader reader = new StringReader("{a:10,b:'c'}");
95143
JsonElement e = JsonParser.parseReader(reader);

0 commit comments

Comments
 (0)