diff --git a/modules/lang-groovy/src/main/java/org/elasticsearch/script/groovy/GroovyScriptEngineService.java b/modules/lang-groovy/src/main/java/org/elasticsearch/script/groovy/GroovyScriptEngineService.java index a83dd93a17e..6216ec2354e 100644 --- a/modules/lang-groovy/src/main/java/org/elasticsearch/script/groovy/GroovyScriptEngineService.java +++ b/modules/lang-groovy/src/main/java/org/elasticsearch/script/groovy/GroovyScriptEngineService.java @@ -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) 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()); } } diff --git a/modules/lang-groovy/src/test/java/org/elasticsearch/script/groovy/GroovySecurityTests.java b/modules/lang-groovy/src/test/java/org/elasticsearch/script/groovy/GroovySecurityTests.java index 96a5343913d..31dc154a9e2 100644 --- a/modules/lang-groovy/src/test/java/org/elasticsearch/script/groovy/GroovySecurityTests.java +++ b/modules/lang-groovy/src/test/java/org/elasticsearch/script/groovy/GroovySecurityTests.java @@ -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 vars = new HashMap(); @@ -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 exceptionClass) { try { doTest(script);