Browse Source

Disable regexes by default in painless

Adds a new node level, non-dynamic setting, `script.painless.regex.enabled`
can be used to enable regexes.

Closes #20397
Nik Everett 9 years ago
parent
commit
69bf08f6c6

+ 3 - 0
docs/build.gradle

@@ -24,6 +24,9 @@ integTest {
     setting 'script.inline', 'true'
     setting 'script.stored', 'true'
     setting 'script.max_compilations_per_minute', '1000'
+    /* Enable regexes in painless so our tests don't complain about example
+     * snippets that use them. */
+    setting 'script.painless.regex.enabled', 'true'
     Closure configFile = {
       extraConfigFile it, "src/test/cluster/config/$it"
     }

+ 9 - 0
docs/reference/modules/scripting/painless.asciidoc

@@ -196,6 +196,15 @@ POST hockey/player/1/_update
 [[modules-scripting-painless-regex]]
 === Regular expressions
 
+NOTE: Regexes are disabled by default because they circumvent Painless's
+protection against long running and memory hungry scripts. To make matters
+worse even innocuous looking regexes can have staggering performance and stack
+depth behavior. They remain an amazing powerful tool but are too scary to enable
+by default. To enable them yourself set `script.painless.regex.enabled: true` in
+`elasticsearch.yml`. We'd like very much to have a safe alternative
+implementation that can be enabled by default so check this space for later
+developments!
+
 Painless's native support for regular expressions has syntax constructs:
 
 * `/pattern/`: Pattern literals create patterns. This is the only way to create

+ 30 - 0
modules/lang-painless/src/main/java/org/elasticsearch/painless/CompilerSettings.java

@@ -19,10 +19,18 @@
 
 package org.elasticsearch.painless;
 
+import org.elasticsearch.common.settings.Setting;
+import org.elasticsearch.common.settings.Setting.Property;
+
 /**
  * Settings to use when compiling a script.
  */
 public final class CompilerSettings {
+    /**
+     * Are regexes enabled? This is a node level setting because regexes break out of painless's lovely sandbox and can cause stack
+     * overflows and we can't analyze the regex to be sure it won't.
+     */
+    public static final Setting<Boolean> REGEX_ENABLED = Setting.boolSetting("script.painless.regex.enabled", false, Property.NodeScope);
 
     /**
      * Constant to be used when specifying the maximum loop counter when compiling a script.
@@ -55,6 +63,12 @@ public final class CompilerSettings {
      */
     private int initialCallSiteDepth = 0;
 
+    /**
+     * Are regexes enabled? They are currently disabled by default because they break out of the loop counter and even fairly simple
+     * <strong>looking</strong> regexes can cause stack overflows.
+     */
+    private boolean regexesEnabled = false;
+
     /**
      * Returns the value for the cumulative total number of statements that can be made in all loops
      * in a script before an exception is thrown.  This attempts to prevent infinite loops.  Note if
@@ -104,4 +118,20 @@ public final class CompilerSettings {
     public void setInitialCallSiteDepth(int depth) {
         this.initialCallSiteDepth = depth;
     }
+
+    /**
+     * Are regexes enabled? They are currently disabled by default because they break out of the loop counter and even fairly simple
+     * <strong>looking</strong> regexes can cause stack overflows.
+     */
+    public boolean areRegexesEnabled() {
+        return regexesEnabled;
+    }
+
+    /**
+     * Are regexes enabled? They are currently disabled by default because they break out of the loop counter and even fairly simple
+     * <strong>looking</strong> regexes can cause stack overflows.
+     */
+    public void setRegexesEnabled(boolean regexesEnabled) {
+        this.regexesEnabled = regexesEnabled;
+    }
 }

+ 10 - 3
modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessPlugin.java

@@ -20,19 +20,21 @@
 package org.elasticsearch.painless;
 
 
+import org.elasticsearch.common.settings.Setting;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.plugins.Plugin;
 import org.elasticsearch.plugins.ScriptPlugin;
-import org.elasticsearch.script.ScriptEngineRegistry;
 import org.elasticsearch.script.ScriptEngineService;
-import org.elasticsearch.script.ScriptModule;
+
+import java.util.Arrays;
+import java.util.List;
 
 /**
  * Registers Painless as a plugin.
  */
 public final class PainlessPlugin extends Plugin implements ScriptPlugin {
 
-    // force to pare our definition at startup (not on the user's first script)
+    // force to parse our definition at startup (not on the user's first script)
     static {
         Definition.VOID_TYPE.hashCode();
     }
@@ -41,4 +43,9 @@ public final class PainlessPlugin extends Plugin implements ScriptPlugin {
     public ScriptEngineService getScriptEngineService(Settings settings) {
         return new PainlessScriptEngineService(settings);
     }
+
+    @Override
+    public List<Setting<?>> getSettings() {
+        return Arrays.asList(CompilerSettings.REGEX_ENABLED);
+    }
 }

+ 18 - 9
modules/lang-painless/src/main/java/org/elasticsearch/painless/PainlessScriptEngineService.java

@@ -53,11 +53,6 @@ public final class PainlessScriptEngineService extends AbstractComponent impleme
      */
     public static final String NAME = "painless";
 
-    /**
-     * Default compiler settings to be used.
-     */
-    private static final CompilerSettings DEFAULT_COMPILER_SETTINGS = new CompilerSettings();
-
     /**
      * Permissions context used during compilation.
      */
@@ -74,12 +69,19 @@ public final class PainlessScriptEngineService extends AbstractComponent impleme
         });
     }
 
+    /**
+     * Default compiler settings to be used. Note that {@link CompilerSettings} is mutable but this instance shouldn't be mutated outside
+     * of {@link PainlessScriptEngineService#PainlessScriptEngineService(Settings)}.
+     */
+    private final CompilerSettings defaultCompilerSettings = new CompilerSettings();
+
     /**
      * Constructor.
      * @param settings The settings to initialize the engine with.
      */
     public PainlessScriptEngineService(final Settings settings) {
         super(settings);
+        defaultCompilerSettings.setRegexesEnabled(CompilerSettings.REGEX_ENABLED.get(settings));
     }
 
     /**
@@ -111,29 +113,36 @@ public final class PainlessScriptEngineService extends AbstractComponent impleme
 
         if (params.isEmpty()) {
             // Use the default settings.
-            compilerSettings = DEFAULT_COMPILER_SETTINGS;
+            compilerSettings = defaultCompilerSettings;
         } else {
             // Use custom settings specified by params.
             compilerSettings = new CompilerSettings();
+
+            // Except regexes enabled - this is a node level setting and can't be changed in the request.
+            compilerSettings.setRegexesEnabled(defaultCompilerSettings.areRegexesEnabled());
+
             Map<String, String> copy = new HashMap<>(params);
-            String value = copy.remove(CompilerSettings.MAX_LOOP_COUNTER);
 
+            String value = copy.remove(CompilerSettings.MAX_LOOP_COUNTER);
             if (value != null) {
                 compilerSettings.setMaxLoopCounter(Integer.parseInt(value));
             }
 
             value = copy.remove(CompilerSettings.PICKY);
-
             if (value != null) {
                 compilerSettings.setPicky(Boolean.parseBoolean(value));
             }
 
             value = copy.remove(CompilerSettings.INITIAL_CALL_SITE_DEPTH);
-
             if (value != null) {
                 compilerSettings.setInitialCallSiteDepth(Integer.parseInt(value));
             }
 
+            value = copy.remove(CompilerSettings.REGEX_ENABLED.getKey());
+            if (value != null) {
+                throw new IllegalArgumentException("[painless.regex.enabled] can only be set on node startup.");
+            }
+
             if (!copy.isEmpty()) {
                 throw new IllegalArgumentException("Unrecognized compile-time parameter(s): " + copy);
             }

+ 5 - 0
modules/lang-painless/src/main/java/org/elasticsearch/painless/antlr/Walker.java

@@ -796,6 +796,11 @@ public final class Walker extends PainlessParserBaseVisitor<ANode> {
 
     @Override
     public ANode visitRegex(RegexContext ctx) {
+        if (false == settings.areRegexesEnabled()) {
+            throw location(ctx).createError(new IllegalStateException("Regexes are disabled. Set [script.painless.regex.enabled] to [true] "
+                    + "in elasticsearch.yaml to allow them. Be careful though, regexes break out of Painless's protection against deep "
+                    + "recursion and long loops."));
+        }
         String text = ctx.REGEX().getText();
         int lastSlash = text.lastIndexOf('/');
         String pattern = text.substring(1, lastSlash);

+ 10 - 1
modules/lang-painless/src/test/java/org/elasticsearch/painless/RegexTests.java

@@ -19,17 +19,26 @@
 
 package org.elasticsearch.painless;
 
+import org.elasticsearch.common.settings.Settings;
+
 import java.nio.CharBuffer;
 import java.util.Arrays;
 import java.util.HashSet;
 import java.util.regex.Pattern;
 import java.util.regex.PatternSyntaxException;
 
-import static java.util.Collections.emptyMap;
 import static java.util.Collections.singletonMap;
 import static org.hamcrest.Matchers.containsString;
 
 public class RegexTests extends ScriptTestCase {
+    @Override
+    protected Settings scriptEngineSettings() {
+        // Enable regexes just for this test. They are disabled by default.
+        return Settings.builder()
+                .put(CompilerSettings.REGEX_ENABLED.getKey(), true)
+                .build();
+    }
+
     public void testPatternAfterReturn() {
         assertEquals(true, exec("return 'foo' ==~ /foo/"));
         assertEquals(false, exec("return 'bar' ==~ /foo/"));

+ 9 - 1
modules/lang-painless/src/test/java/org/elasticsearch/painless/ScriptTestCase.java

@@ -45,7 +45,14 @@ public abstract class ScriptTestCase extends ESTestCase {
 
     @Before
     public void setup() {
-        scriptEngine = new PainlessScriptEngineService(Settings.EMPTY);
+        scriptEngine = new PainlessScriptEngineService(scriptEngineSettings());
+    }
+
+    /**
+     * Settings used to build the script engine. Override to customize settings like {@link RegexTests} does to enable regexes.
+     */
+    protected Settings scriptEngineSettings() {
+        return Settings.EMPTY;
     }
 
     /** Compiles and returns the result of {@code script} */
@@ -71,6 +78,7 @@ public abstract class ScriptTestCase extends ESTestCase {
         if (picky) {
             CompilerSettings pickySettings = new CompilerSettings();
             pickySettings.setPicky(true);
+            pickySettings.setRegexesEnabled(CompilerSettings.REGEX_ENABLED.get(scriptEngineSettings()));
             Walker.buildPainlessTree(getTestName(), script, pickySettings, null);
         }
         // test actual script execution

+ 13 - 2
modules/lang-painless/src/test/java/org/elasticsearch/painless/WhenThingsGoWrongTests.java

@@ -20,14 +20,13 @@
 package org.elasticsearch.painless;
 
 import org.apache.lucene.util.Constants;
-import org.elasticsearch.script.ScriptException;
 
 import java.lang.invoke.WrongMethodTypeException;
 import java.util.Arrays;
 import java.util.Collections;
 
 import static java.util.Collections.emptyMap;
-import static org.hamcrest.Matchers.containsString;
+import static java.util.Collections.singletonMap;
 
 public class WhenThingsGoWrongTests extends ScriptTestCase {
     public void testNullPointer() {
@@ -234,4 +233,16 @@ public class WhenThingsGoWrongTests extends ScriptTestCase {
             exec("void recurse(int x, int y) {recurse(x, y)} recurse(1, 2);");
         });
     }
+
+    public void testRegexDisabledByDefault() {
+        IllegalStateException e = expectThrows(IllegalStateException.class, () -> exec("return 'foo' ==~ /foo/"));
+        assertEquals("Regexes are disabled. Set [script.painless.regex.enabled] to [true] in elasticsearch.yaml to allow them. "
+                + "Be careful though, regexes break out of Painless's protection against deep recursion and long loops.", e.getMessage());
+    }
+
+    public void testCanNotOverrideRegexEnabled() {
+        IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
+                () -> exec("", null, singletonMap(CompilerSettings.REGEX_ENABLED.getKey(), "true"), null, false));
+        assertEquals("[painless.regex.enabled] can only be set on node startup.", e.getMessage());
+    }
 }

+ 33 - 0
modules/lang-painless/src/test/resources/rest-api-spec/test/plan_a/40_disabled.yaml

@@ -0,0 +1,33 @@
+---
+"Regex in update fails":
+
+  - do:
+      index:
+          index:  test_1
+          type:   test
+          id:     1
+          body:
+              foo:    bar
+              count:  1
+
+  - do:
+      catch: /Regexes are disabled. Set \[script.painless.regex.enabled\] to \[true\] in elasticsearch.yaml to allow them. Be careful though, regexes break out of Painless's protection against deep recursion and long loops./
+      update:
+          index:  test_1
+          type:   test
+          id:     1
+          body:
+            script:
+              lang:   painless
+              inline: "ctx._source.foo = params.bar ==~ /cat/"
+              params: { bar: 'xxx' }
+
+---
+"Regex enabled is not a dynamic setting":
+
+  - do:
+      catch: /setting \[script.painless.regex.enabled\], not dynamically updateable/
+      cluster.put_settings:
+        body:
+          transient:
+            script.painless.regex.enabled: true