fix: check depth of embedded object and array

fixe APIM-1614
gravitee-io · May 12, 2023 · 75287f5 · 75287f5
1 parent a43c997
commit 75287f5
Show file tree

Hide file tree

Showing 7 changed files with 89 additions and 1 deletion.
diff --git a/README.adoc b/README.adoc
@@ -65,3 +65,7 @@ You can configure the policy with the following options:
 | The transformation cannot be executed properly
 
 |===
+
+=== Nested objects
+
+To limit the processing time in case of nested object, a default max depth of nested object has been defined to 1000. This default value can be overriden using the environment variable `gravitee_policy_json_xml_maxdepth`.
diff --git a/src/main/java/io/gravitee/policy/json2xml/transformer/JSONArray.java b/src/main/java/io/gravitee/policy/json2xml/transformer/JSONArray.java
@@ -139,6 +139,7 @@ public JSONArray(JSONTokener x) throws JSONException {
                         x.back();
                         break;
                     case ']':
+                        x.decrementArrayDepth();
                         return;
                     default:
                         throw x.syntaxError("Expected a ',' or ']'");

diff --git a/src/main/java/io/gravitee/policy/json2xml/transformer/JSONObject.java b/src/main/java/io/gravitee/policy/json2xml/transformer/JSONObject.java
@@ -208,6 +208,7 @@ public JSONObject(JSONTokener x) throws JSONException {
                 case 0:
                     throw x.syntaxError("A JSONObject text must end with '}'");
                 case '}':
+                    x.decrementObjectDepth();
                     return;
                 default:
                     x.back();
@@ -233,6 +234,7 @@ public JSONObject(JSONTokener x) throws JSONException {
                     x.back();
                     break;
                 case '}':
+                    x.decrementObjectDepth();
                     return;
                 default:
                     throw x.syntaxError("Expected a ',' or '}'");

diff --git a/src/main/java/io/gravitee/policy/json2xml/transformer/JSONTokener.java b/src/main/java/io/gravitee/policy/json2xml/transformer/JSONTokener.java
@@ -15,7 +15,15 @@
  */
 package io.gravitee.policy.json2xml.transformer;
 
-import java.io.*;
+import static java.lang.System.getenv;
+import static java.util.Optional.ofNullable;
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.InputStreamReader;
+import java.io.Reader;
+import java.io.StringReader;
 
 /*
 Copyright (c) 2002 JSON.org
@@ -50,6 +58,8 @@ of this software and associated documentation files (the "Software"), to deal
  */
 public class JSONTokener {
 
+    public static final int DEFAULT_MAX_DEPTH = 1000;
+    public static final String GRAVITEE_POLICY_JSON_XML_MAXDEPTH = "gravitee_policy_json_xml_maxdepth";
     private long character;
     private boolean eof;
     private long index;
@@ -58,6 +68,10 @@ public class JSONTokener {
     private Reader reader;
     private boolean usePrevious;
 
+    private int objectDepth = 0;
+    private int arrayDepth = 0;
+    private final int maxDepth;
+
     /**
      * Construct a JSONTokener from a Reader.
      *
@@ -71,6 +85,7 @@ public JSONTokener(Reader reader) {
         this.index = 0;
         this.character = 1;
         this.line = 1;
+        this.maxDepth = ofNullable(getenv(GRAVITEE_POLICY_JSON_XML_MAXDEPTH)).map(Integer::parseInt).orElse(DEFAULT_MAX_DEPTH);
     }
 
     /**
@@ -336,6 +351,14 @@ public String nextTo(String delimiters) throws JSONException {
         }
     }
 
+    public final void decrementObjectDepth() {
+        this.objectDepth--;
+    }
+
+    public final void decrementArrayDepth() {
+        this.arrayDepth--;
+    }
+
     /**
      * Get the next value. The value can be a Boolean, Double, Integer,
      * JSONArray, JSONObject, Long, or String, or the JSONObject.NULL object.
@@ -353,9 +376,11 @@ public Object nextValue() throws JSONException {
                 return this.nextString(c);
             case '{':
                 this.back();
+                checkMaxDepth(this.objectDepth++);
                 return new JSONObject(this);
             case '[':
                 this.back();
+                checkMaxDepth(this.arrayDepth++);
                 return new JSONArray(this);
         }
 
@@ -382,6 +407,12 @@ public Object nextValue() throws JSONException {
         return JSONObject.stringToValue(string);
     }
 
+    private void checkMaxDepth(int depth) {
+        if (depth > this.maxDepth && this.maxDepth > -1) {
+            throw new IllegalArgumentException("Too many nested objects or arrays");
+        }
+    }
+
     /**
      * Skip characters until the next character is the requested character.
      * If the requested character is not found, no characters are skipped.

diff --git a/src/test/java/io/gravitee/policy/json2xml/JsonToXmlTransformationPolicyTest.java b/src/test/java/io/gravitee/policy/json2xml/JsonToXmlTransformationPolicyTest.java
@@ -120,6 +120,54 @@ public void shouldNotTransformAndAddHeadersOnRequestContent() throws Exception {
         verify(policyChain, times(1)).streamFailWith(any());
     }
 
+    @Test
+    @DisplayName("Should throw TransformationException if maxDepth reach for object")
+    public void shouldLimitObjectDepthOnRequestContent() throws Exception {
+        String input = loadResource("/io/gravitee/policy/json2xml/invalid-embedded-object.json");
+
+        // Prepare context
+        when(configuration.getScope()).thenReturn(PolicyScope.REQUEST);
+        when(request.headers()).thenReturn(HttpHeaders.create());
+        when(request.metrics()).thenReturn(Metrics.on(Instant.now().toEpochMilli()).build());
+
+        final ReadWriteStream result = cut.onRequestContent(request, policyChain);
+        assertThat(result).isNotNull();
+
+        result.write(Buffer.buffer(input));
+        result.end();
+
+        assertThat(request.headers().names()).doesNotContain(HttpHeaderNames.CONTENT_TYPE);
+        assertThat(request.headers().names()).doesNotContain(HttpHeaderNames.TRANSFER_ENCODING);
+        assertThat(request.headers().names()).doesNotContain(HttpHeaderNames.CONTENT_LENGTH);
+        assertThat(request.metrics().getMessage()).contains("Unable to transform JSON into XML:");
+        assertThat(request.metrics().getMessage()).contains("Too many nested objects or arrays");
+        verify(policyChain, times(1)).streamFailWith(any());
+    }
+
+    @Test
+    @DisplayName("Should throw TransformationException if maxDepth reach for array")
+    public void shouldLimitArrayDepthOnRequestContent() throws Exception {
+        String input = loadResource("/io/gravitee/policy/json2xml/invalid-embedded-array.json");
+
+        // Prepare context
+        when(configuration.getScope()).thenReturn(PolicyScope.REQUEST);
+        when(request.headers()).thenReturn(HttpHeaders.create());
+        when(request.metrics()).thenReturn(Metrics.on(Instant.now().toEpochMilli()).build());
+
+        final ReadWriteStream result = cut.onRequestContent(request, policyChain);
+        assertThat(result).isNotNull();
+
+        result.write(Buffer.buffer(input));
+        result.end();
+
+        assertThat(request.headers().names()).doesNotContain(HttpHeaderNames.CONTENT_TYPE);
+        assertThat(request.headers().names()).doesNotContain(HttpHeaderNames.TRANSFER_ENCODING);
+        assertThat(request.headers().names()).doesNotContain(HttpHeaderNames.CONTENT_LENGTH);
+        assertThat(request.metrics().getMessage()).contains("Unable to transform JSON into XML:");
+        assertThat(request.metrics().getMessage()).contains("Too many nested objects or arrays");
+        verify(policyChain, times(1)).streamFailWith(any());
+    }
+
     @Test
     @DisplayName("Should transform and add header OnResponseContent")
     public void shouldTransformAndAddHeadersOnResponseContent() throws Exception {

diff --git a/src/test/resources/io/gravitee/policy/json2xml/invalid-embedded-array.json b/src/test/resources/io/gravitee/policy/json2xml/invalid-embedded-array.json
@@ -0,0 +1 @@
+{"a":[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[
diff --git a/src/test/resources/io/gravitee/policy/json2xml/invalid-embedded-object.json b/src/test/resources/io/gravitee/policy/json2xml/invalid-embedded-object.json
@@ -0,0 +1 @@
+{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		{"a":[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[[
Original file line number	Diff line number	Diff line change
		@@ -0,0 +1 @@
		{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":{"a":