Fix #3328 (improvement to handling of JDK serialization of JsonNode)

FasterXML · Dec 4, 2021 · 3ccde7d · 3ccde7d
1 parent 70ba54f
commit 3ccde7d
Show file tree

Hide file tree

Showing 3 changed files with 76 additions and 6 deletions.
diff --git a/release-notes/VERSION-2.x b/release-notes/VERSION-2.x
@@ -9,6 +9,7 @@ Project: jackson-databind
 #3280: Can not deserialize json to enum value with Object-/Array-valued input,
   `@JsonCreator`
  (reported by peteryuanpan@github)
+#3328: Possible DoS issue
 
 2.12.5 (27-Aug-2021)
 

diff --git a/src/main/java/com/fasterxml/jackson/databind/node/NodeSerialization.java b/src/main/java/com/fasterxml/jackson/databind/node/NodeSerialization.java
@@ -4,6 +4,8 @@
 import java.io.ObjectInput;
 import java.io.ObjectOutput;
 
+import com.fasterxml.jackson.core.util.ByteArrayBuilder;
+
 /**
  * Helper value class only used during JDK serialization: contains JSON as `byte[]`
  *
@@ -12,6 +14,9 @@
 class NodeSerialization implements java.io.Serializable,
     java.io.Externalizable
 {
+    // To avoid malicious input only allocate up to 100k
+    protected final static int LONGEST_EAGER_ALLOC = 100_000;
+
     private static final long serialVersionUID = 1L;
 
     public byte[] json;
@@ -45,7 +50,36 @@ public void writeExternal(ObjectOutput out) throws IOException {
     @Override
     public void readExternal(ObjectInput in) throws IOException {
         final int len = in.readInt();
-        json = new byte[len];
-        in.readFully(json, 0, len);
+        json = _read(in, len);
+    }
+
+    private byte[] _read(ObjectInput in, int expLen) throws IOException {
+        // Common case, just read directly
+        if (expLen <= LONGEST_EAGER_ALLOC) {
+            byte[] result = new byte[expLen];
+            in.readFully(result, 0, expLen);
+            return result;
+        }
+        // but longer content needs more care to avoid DoS by maliciously crafted data
+        // (this wrt [databind#3328]
+        try (final ByteArrayBuilder bb = new ByteArrayBuilder(LONGEST_EAGER_ALLOC)) {
+            byte[] buffer = bb.resetAndGetFirstSegment();
+            int outOffset = 0;
+            while (true) {
+                int toRead = Math.min(buffer.length - outOffset, expLen);
+                in.readFully(buffer, 0, toRead);
+                expLen -= toRead;
+                outOffset += toRead;
+                // Did we get everything we needed? If so, we are done
+                if (expLen == 0) {
+                    return bb.completeAndCoalesce(outOffset);
+                }
+                // Or perhaps we filled the current segment? If so, finish, get next
+                if (outOffset == buffer.length) {
+                    buffer = bb.finishCurrentSegment();
+                    outOffset = 0;
+                }
+            }
+        }
     }
 }
diff --git a/...on/databind/TestNodeJDKSerialization.java → ...tabind/node/NodeJDKSerializationTest.java b/...on/databind/TestNodeJDKSerialization.java → ...tabind/node/NodeJDKSerializationTest.java
@@ -1,11 +1,14 @@
-package com.fasterxml.jackson.databind;
+package com.fasterxml.jackson.databind.node;
 
 import java.io.*;
 
-import com.fasterxml.jackson.databind.node.ArrayNode;
-import com.fasterxml.jackson.databind.node.ObjectNode;
+import com.fasterxml.jackson.core.JsonGenerator;
 
-public class TestNodeJDKSerialization extends BaseMapTest
+import com.fasterxml.jackson.databind.BaseMapTest;
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+
+public class NodeJDKSerializationTest extends BaseMapTest
 {
     private final ObjectMapper MAPPER = newJsonMapper();
 
@@ -40,6 +43,38 @@ public void testArrayNodeSerialization() throws Exception
         testNodeRoundtrip(root);
     }
 
+    // [databind#3328]
+    public void testBigArrayNodeSerialization() throws Exception
+    {
+        // Try couple of variations just to tease out possible edge cases
+        _testBigArrayNodeSerialization(NodeSerialization.LONGEST_EAGER_ALLOC - 39);
+        _testBigArrayNodeSerialization(NodeSerialization.LONGEST_EAGER_ALLOC + 1);
+        _testBigArrayNodeSerialization(3 * NodeSerialization.LONGEST_EAGER_ALLOC - 1);
+        _testBigArrayNodeSerialization(9 * NodeSerialization.LONGEST_EAGER_ALLOC);
+    }
+
+    private void _testBigArrayNodeSerialization(int expSize) throws Exception
+    {
+        ByteArrayOutputStream out = new ByteArrayOutputStream();
+        int ix = 0;
+        try (JsonGenerator g = MAPPER.createGenerator(out)) {
+            g.writeStartArray();
+
+            do {
+                g.writeStartObject();
+                g.writeNumberField("index", ix++);
+                g.writeStringField("extra", "none#"+ix);
+                g.writeEndObject();
+            } while (out.size() < expSize);
+
+            g.writeEndArray();
+        }
+
+        JsonNode root = MAPPER.readTree(out.toByteArray());
+
+        testNodeRoundtrip(root);
+    }
+
     // and then also some scalar types
     public void testScalarSerialization() throws Exception
     {