Stack overflow error caused by gson serialization Map

[PoppingSnack]

Stack overflow error caused by gson serialization Map

Description

gson before v2.10.1 was discovered to contain a stack overflow via the map parameter. This vulnerability allows attackers to cause a Denial of Service (DoS) via a crafted string.

Error Log

Exception in thread "main" java.lang.StackOverflowError
	at java.base/java.lang.AbstractStringBuilder.appendChars(AbstractStringBuilder.java:1693)
	at java.base/java.lang.AbstractStringBuilder.append(AbstractStringBuilder.java:634)
	at java.base/java.lang.StringBuffer.append(StringBuffer.java:397)
	at java.base/java.io.StringWriter.write(StringWriter.java:122)
	at com.google.gson.stream.JsonWriter.string(JsonWriter.java:642)
	at com.google.gson.stream.JsonWriter.writeDeferredName(JsonWriter.java:402)
	at com.google.gson.stream.JsonWriter.beginObject(JsonWriter.java:311)
	at com.google.gson.internal.bind.MapTypeAdapterFactory$Adapter.write(MapTypeAdapterFactory.java:204)
	at com.google.gson.internal.bind.MapTypeAdapterFactory$Adapter.write(MapTypeAdapterFactory.java:144)
	at com.google.gson.internal.bind.TypeAdapterRuntimeTypeWrapper.write(TypeAdapterRuntimeTypeWrapper.java:70)
	at com.google.gson.internal.bind.MapTypeAdapterFactory$Adapter.write(MapTypeAdapterFactory.java:207)
	at com.google.gson.internal.bind.MapTypeAdapterFactory$Adapter.write(MapTypeAdapterFactory.java:144)
	at com.google.gson.internal.bind.TypeAdapterRuntimeTypeWrapper.write(TypeAdapterRuntimeTypeWrapper.java:70)
	at com.google.gson.internal.bind.MapTypeAdapterFactory$Adapter.write(MapTypeAdapterFactory.java:207)
	at com.google.gson.internal.bind.MapTypeAdapterFactory$Adapter.write(MapTypeAdapterFactory.java:144)
	at com.google.gson.internal.bind.TypeAdapterRuntimeTypeWrapper.write(TypeAdapterRuntimeTypeWrapper.java:70)
	at com.google.gson.internal.bind.MapTypeAdapterFactory$Adapter.write(MapTypeAdapterFactory.java:207)
	at com.google.gson.internal.bind.MapTypeAdapterFactory$Adapter.write(MapTypeAdapterFactory.java:144)
	at com.google.gson.internal.bind.TypeAdapterRuntimeTypeWrapper.write(TypeAdapterRuntimeTypeWrapper.java:70)
	at com.google.gson.internal.bind.MapTypeAdapterFactory$Adapter.write(MapTypeAdapterFactory.java:207)
	at com.google.gson.internal.bind.MapTypeAdapterFactory$Adapter.write(MapTypeAdapterFactory.java:144)
	at com.google.gson.internal.bind.TypeAdapterRuntimeTypeWrapper.write(TypeAdapterRuntimeTypeWrapper.java:70)
	at com.google.gson.internal.bind.MapTypeAdapterFactory$Adapter.write(MapTypeAdapterFactory.java:207)
	at com.google.gson.internal.bind.MapTypeAdapterFactory$Adapter.write(MapTypeAdapterFactory.java:144)

PoC

        <dependency>
            <groupId>com.google.code.gson</groupId>
            <artifactId>gson</artifactId>
            <version>2.10.1</version>
        </dependency>

import com.google.gson.Gson;

import java.util.HashMap;

public class PoC2 {

    public static void main(String[] args) {
        HashMap<String,Object> map=new HashMap<>();
        map.put("t",map);
        Gson gson = new Gson();
        String jsonString = gson.toJson(map);
    }
}

Rectification Solution

1. Refer to the solution of jackson-databind: Add the depth variable to record the current parsing depth. If the parsing depth exceeds a certain threshold, an exception is thrown. (FasterXML/jackson-databind@fcfc499)

2. Refer to the GSON solution: Change the recursive processing on deeply nested arrays or JSON objects to stack+iteration processing.(（2d01d6a20f39881c692977564c1ea591d9f39027）)

References

1. If the value in map is the map's self, the new new JSONObject(map) cause StackOverflowError which may lead to dos jettison-json/jettison#52
2. https://github.com/jettison-json/jettison/pull/53/files

[Marcono1234]

Thanks for the report! In general though, please report security vulnerabilities as described in https://github.com/google/gson/security; though in this case I guess this might not be that severe, see below.

This vulnerability allows attackers to cause a Denial of Service (DoS) via a crafted string.

Could you please explain this a bit more in detail? Your proof of concept shows Java code which triggers this issue, not a JSON string, so if an attacker can already execute arbitrary Java code, they could instead directly run OS commands.
Maybe there are some corner cases where applications indirectly (e.g. through a configuration UI) allow nesting the same objects (or deeply nesting different objects), but that would affect those specific applications and not apply to all applications.

Either way, personally I think it would be good to improve this handling of deeply nested values (collections, maps, objects) either way, also if possible to make debugging easier, because users sometimes by accident create recursive data structures, and debugging the StackOverflowError might not be that easy. But I am not sure what the maintainers think.

(same applies to #2417 as well)

This relates to #530

Edit: But in general such recursive values as shown in your sample code will cause problems, not only for Gson. For example calling hashCode() on map in your example will trigger a StackOverflowError as well. And if you introduce a layer of indirection (map -> map2 -> map) and then call map.toString() you will encounter a StackOverflowError too.

[Marcono1234]

Or maybe the simplest solution would be to introduce a maximum nesting depth for JsonReader and JsonWriter. That should solve this issue as well as a potential StackOverflowError when deserializing recursive data structures (e.g. class Node { Node nested; }).
Maybe there should be a reasonable default limit which prevents a StackOverflowError in most cases, but users can adjust it if necessary (maybe also through a GsonBuilder method which affects both serialization and deserialization?).

[pjfanning]

@Marcono1234 this user (PoppingSnack) has been irresponsibly raising issues across a large number of Java JSON tools (including Jackson, a lib that I work on). Most or all of these tools having documented approaches on how to report issues responsibly and this user has ignored them all.

If there is any attempt by this user to claim credit and cash rewards for disclosing vulnerabilities, I hope that the GSON team will not give the user credit due to the irresponsible disclosure.

[eamonnmcmanus]

If you make a recursive data structure and try to serialize it, you get a StackOverflowError. As @Marcono1234 points out, you'll also get a StackOverflowError if you call hashCode() on it. This doesn't seem like any sort of a security problem, and I'm not sure it is worth defending against this particular sort of programmer error.