Browse Source

Limit the depth of a filter (#133113)

This commit limits the maximum depth of a filter expression such as
`abc.*.d*.*e.f.*`

These expressions are most commonly used in `filter_path` on in a URL,
and the limit has been set to 500 - in effect such a filter may only
contain 500 `.` separators between field expressions.
Tim Vernum 1 month ago
parent
commit
c904039f71

+ 5 - 0
docs/changelog/133113.yaml

@@ -0,0 +1,5 @@
+pr: 133113
+summary: Limit the depth of a filter
+area: Infra/REST API
+type: enhancement
+issues: []

+ 12 - 3
libs/x-content/src/main/java/org/elasticsearch/xcontent/support/filtering/FilterPath.java

@@ -22,6 +22,9 @@ public class FilterPath {
     private static final String WILDCARD = "*";
     private static final String DOUBLE_WILDCARD = "**";
 
+    // This is ridiculously large, but we can be 100% certain that if any filter tries to exceed this depth then it is a mistake
+    static final int MAX_TREE_DEPTH = 500;
+
     private final Map<String, FilterPath> termsChildren;
     private final FilterPath[] wildcardChildren;
     private final String pattern;
@@ -132,6 +135,7 @@ public class FilterPath {
     }
 
     private static class FilterPathBuilder {
+
         private static class BuildNode {
             private final Map<String, BuildNode> children;
             private final boolean isFinalNode;
@@ -145,14 +149,19 @@ public class FilterPath {
         private final BuildNode root = new BuildNode(false);
 
         void insert(String filter) {
-            insertNode(filter, root);
+            insertNode(filter, root, 0);
         }
 
         FilterPath build() {
             return buildPath("", root);
         }
 
-        static void insertNode(String filter, BuildNode node) {
+        static void insertNode(String filter, BuildNode node, int depth) {
+            if (depth > MAX_TREE_DEPTH) {
+                throw new IllegalArgumentException(
+                    "Filter exceeds maximum depth at [" + (filter.length() > 100 ? filter.substring(0, 100) : filter) + "]"
+                );
+            }
             int end = filter.length();
             int splitPosition = -1;
             boolean findEscapes = false;
@@ -171,7 +180,7 @@ public class FilterPath {
                 String field = findEscapes ? filter.substring(0, splitPosition).replace("\\.", ".") : filter.substring(0, splitPosition);
                 BuildNode child = node.children.computeIfAbsent(field, f -> new BuildNode(false));
                 if (false == child.isFinalNode) {
-                    insertNode(filter.substring(splitPosition + 1), child);
+                    insertNode(filter.substring(splitPosition + 1), child, depth + 1);
                 }
             } else {
                 String field = findEscapes ? filter.replace("\\.", ".") : filter;

+ 13 - 0
libs/x-content/src/test/java/org/elasticsearch/xcontent/support/filtering/FilterPathTests.java

@@ -21,6 +21,7 @@ import java.util.stream.Collectors;
 
 import static java.util.Collections.singleton;
 import static org.hamcrest.Matchers.arrayWithSize;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.is;
 
 public class FilterPathTests extends ESTestCase {
@@ -403,4 +404,16 @@ public class FilterPathTests extends ESTestCase {
         assertTrue(filterPaths[0].matches("a.b.c.d", nextFilters, true));
         assertEquals(nextFilters.size(), 0);
     }
+
+    public void testDepthChecking() {
+        final String atLimit = "x" + (".x").repeat(FilterPath.MAX_TREE_DEPTH);
+        final String aboveLimit = atLimit + ".y";
+
+        var paths = FilterPath.compile(Set.of(atLimit));
+        assertThat(paths, arrayWithSize(1));
+
+        var ex = expectThrows(IllegalArgumentException.class, () -> FilterPath.compile(Set.of(aboveLimit)));
+        assertThat(ex.getMessage(), containsString("maximum depth"));
+        assertThat(ex.getMessage(), containsString("[y]"));
+    }
 }