honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge remote-tracking branch 'dakrone/handle-groovy-errors-better'

This commit is contained in:
Lee Hinman 2016-08-12 13:07:13 -06:00
parent efc32746eb aab8c1f032
commit f832eaa6d2
2 changed files with 20 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
modules/lang-groovy/src/main/java/org/elasticsearch/script/groovy/GroovyScriptEngineService.java
View File

@ -293,10 +293,19 @@ public class GroovyScriptEngineService extends AbstractComponent implements Scri
// NOTE: we truncate the stack because IndyInterface has security issue (needs getClassLoader)
// we don't do a security check just as a tradeoff, it cannot really escalate to anything.
return AccessController.doPrivileged((PrivilegedAction<Object>) script::run);
} catch (Exception e) {
if (logger.isTraceEnabled()) {
logger.trace("failed to run {}", e, compiledScript);
} catch (AssertionError ae) {
// Groovy asserts are not java asserts, and cannot be disabled, so we do a best-effort trying to determine if this is a
// Groovy assert (in which case we wrap it and throw), or a real Java assert, in which case we rethrow it as-is, likely
// resulting in the uncaughtExceptionHandler handling it.
final StackTraceElement[] elements = ae.getStackTrace();
if (elements.length > 0 && "org.codehaus.groovy.runtime.InvokerHelper".equals(elements[0].getClassName())) {
logger.trace("failed to run {}", ae, compiledScript);
throw new ScriptException("Error evaluating " + compiledScript.name(),
ae, emptyList(), "", compiledScript.lang());
}
throw ae;
} catch (Exception | NoClassDefFoundError e) {
logger.trace("failed to run {}", e, compiledScript);
throw new ScriptException("Error evaluating " + compiledScript.name(), e, emptyList(), "", compiledScript.lang());
}
}

9
modules/lang-groovy/src/test/java/org/elasticsearch/script/groovy/GroovySecurityTests.java
View File

@ -123,6 +123,13 @@ public class GroovySecurityTests extends ESTestCase {
}
}
public void testGroovyScriptsThatThrowErrors() throws Exception {
assertFailure("assert false, \"msg\";", AssertionError.class);
assertFailure("def foo=false; assert foo;", AssertionError.class);
// Groovy's asserts require org.codehaus.groovy.runtime.InvokerHelper, so they are denied
assertFailure("def foo=false; assert foo, \"msg2\";", NoClassDefFoundError.class);
}
/** runs a script */
private void doTest(String script) {
Map<String, Object> vars = new HashMap<String, Object>();
@ -146,7 +153,7 @@ public class GroovySecurityTests extends ESTestCase {
doTest(script);
}
/** asserts that a script triggers securityexception */
/** asserts that a script triggers the given exceptionclass */
private void assertFailure(String script, Class<? extends Throwable> exceptionClass) {
try {
doTest(script);