Skip to content

Commit

Permalink
Disable regexes by default in painless
Browse files Browse the repository at this point in the history
Adds a new node level, non-dynamic setting, `script.painless.regex.enabled`
can be used to enable regexes.

Closes #20397
  • Loading branch information
nik9000 committed Sep 12, 2016
1 parent 53222a8 commit 1bd9905
Show file tree
Hide file tree
Showing 10 changed files with 140 additions and 16 deletions.
3 changes: 3 additions & 0 deletions docs/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
Expand Down
9 changes: 9 additions & 0 deletions docs/reference/modules/scripting/painless.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -104,4 +118,20 @@ public int getInitialCallSiteDepth() {
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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand All @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand All @@ -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));
}

/**
Expand Down Expand Up @@ -111,29 +113,36 @@ public Object compile(String scriptName, final String scriptSource, final Map<St

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);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -796,6 +796,11 @@ public ANode visitString(StringContext ctx) {

@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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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/"));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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} */
Expand All @@ -71,6 +78,7 @@ public Object exec(String script, Map<String, Object> vars, Map<String,String> c
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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -234,4 +233,16 @@ public void testStackOverflowError() {
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());
}
}
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 1bd9905

Please sign in to comment.