Catch AssertionError and NoClassDefFoundError in groovy scripts

Previously, we only caught subclasses of Exception, however, there are some cases when Errors are thrown instead of Exceptions. These two cases are `assert` and when a class cannot be found. Without this change, the exception would bubble up to the `uncaughtExceptionHandler`, which would in turn, exit the JVM (related: elastic#19923). A note of difference between regular Java asserts and Groovy asserts, from http://docs.groovy-lang.org/docs/latest/html/documentation/core-testing-guide.html "Another important difference from Java is that in Groovy assertions are enabled by default. It has been a language design decision to remove the possibility to deactivate assertions." In the event that a user uses an assert such as: ```groovy def bar=false; assert bar, "message"; ``` The GroovyScriptEngineService throws a NoClassDefFoundError being unable to find the `java.lang.StringBuffer` class. It is *highly* recommended that any Groovy scripting user switch to using regular exceptions rather than unconfiguration Groovy assertions. Resolves elastic#19806

Author: dakrone

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

@@ -293,10 +293,19 @@ public Object run() {
  // 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());
  }
  }

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

@@ -123,6 +123,13 @@ public void testEvilGroovyScripts() throws Exception {
  }
  }
 
+ 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 @@ private void assertSuccess(String script) {
  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);