This is an automated email from the ASF dual-hosted git repository. henrib pushed a commit to branch JEXL-468 in repository https://gitbox.apache.org/repos/asf/commons-jexl.git
commit 5f4c69acff1dbcb91b56e35c11e3094dbd5d9e0b Author: Henrib <[email protected]> AuthorDate: Thu Aug 20 11:42:21 2026 +0200 [JEXL-468] Sandbox iterator gate, permission-parser polarity, engine-compiler denial f006: SandboxUberspect.getIterator delegated straight to the base uberspect, bypassing the sandbox. Route iteration through the sandbox by consulting the "iterator" method permission for the object's class; deny (null iterator, an empty loop) when it is blocked. f008: PermissionsParser mixed +/- polarity handling. A '+' before a member mutated the class-scoped deny flag even after the class was created, leaking the flipped polarity into a following inner class; an inner-class sign was discarded (the outer polarity leaked in instead). Now '+' only selects an allowing class when it prefixes the class name, an inner class takes its own polarity from its explicit sign, and mixing +/- on one element is a parse error. f009/f036: deny the second-stage compiler surface under RESTRICTED - the createScript/createExpression/createJxltEngine methods on JexlEngine, the getThreadEngine/setThreadContext thread-locals, and createExpression/ createTemplate on JxltEngine - so a script holding a live engine cannot compile and run further scripts. Document the lambda-invocation bypass caveat. f007: document the JexlArithmetic operator fast-path exemptions (equals, compareTo, toString, isEmpty, size, contains are invoked directly, not through the permission-gated uberspect) on the JexlPermissions javadoc. Co-Authored-By: Claude Opus 4.8 <[email protected]> --- .../internal/introspection/PermissionsParser.java | 27 +++++++++++--- .../internal/introspection/SandboxUberspect.java | 11 +++++- .../jexl3/introspection/JexlPermissions.java | 22 +++++++++++- .../internal/introspection/PermissionsTest.java | 42 +++++++++++++++++++++- .../commons/jexl3/introspection/SandboxTest.java | 28 +++++++++++++++ 5 files changed, 122 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/apache/commons/jexl3/internal/introspection/PermissionsParser.java b/src/main/java/org/apache/commons/jexl3/internal/introspection/PermissionsParser.java index 1ddcac2a..4ffd4efc 100644 --- a/src/main/java/org/apache/commons/jexl3/internal/introspection/PermissionsParser.java +++ b/src/main/java/org/apache/commons/jexl3/internal/introspection/PermissionsParser.java @@ -136,7 +136,8 @@ public class PermissionsParser { String identifier = inner; boolean deny = nojexl; boolean classPositive = false; // the class's own polarity (set at creation, never mutated) - boolean memberNegative = false; // whether the pending member is prefixed with '-' + boolean memberNegative = false; // whether the pending member/inner class is prefixed with '-' + boolean memberPositive = false; // whether the pending member/inner class is prefixed with '+' boolean hasNested = false; // whether this (otherwise empty) class only encloses nested classes int i = offset; int j = -1; @@ -165,13 +166,24 @@ public class PermissionsParser { } // read an identifier, the class name if (identifier == null) { - // negative or positive set ? + // negative or positive set ? mixing signs on the same element is an error if (c == '-') { + if (memberPositive) { + throw new IllegalStateException(unexpected(c, i)); + } // a '-' before a member denies it; tracked for a possible deny-list upgrade memberNegative = true; i += 1; } else if (c == '+') { - deny = false; + if (memberNegative) { + throw new IllegalStateException(unexpected(c, i)); + } + memberPositive = true; + // a '+' only selects an allowing class when it prefixes the class name; once the class + // exists it must not flip the class polarity (which would leak into inner classes) + if (njclass == null) { + deny = false; + } i += 1; } final int next = readIdentifier(temp, i); @@ -192,16 +204,20 @@ public class PermissionsParser { njclass = deny ? new Permissions.NoJexlClass() : new Permissions.JexlClass(); classPositive = !deny; memberNegative = false; // a class-level sign does not carry to members + memberPositive = false; njname = outer != null ? outer + "$" + identifier : identifier; njpackage.addNoJexl(njname, njclass); identifier = null; } else if (identifier != null) { // class member mode if (c == '{') { - // inner class - i = readClass(njpackage, deny, njname, identifier, i - 1); + // inner class: its own polarity comes from its explicit sign ('-' deny, '+' allow), + // defaulting to the enclosing class's polarity; the sign never mutates the outer class + final boolean innerDeny = memberNegative || !memberPositive && !classPositive; + i = readClass(njpackage, innerDeny, njname, identifier, i - 1); identifier = null; memberNegative = false; // an inner-class sign does not change the outer class + memberPositive = false; hasNested = true; // this class encloses at least one nested class declaration continue; } @@ -222,6 +238,7 @@ public class PermissionsParser { } identifier = null; memberNegative = false; + memberPositive = false; } else if (c == '(' && !isMethod) { // method; only one opening parenthesis allowed isMethod = true; diff --git a/src/main/java/org/apache/commons/jexl3/internal/introspection/SandboxUberspect.java b/src/main/java/org/apache/commons/jexl3/internal/introspection/SandboxUberspect.java index 81de6b93..7c7d865c 100644 --- a/src/main/java/org/apache/commons/jexl3/internal/introspection/SandboxUberspect.java +++ b/src/main/java/org/apache/commons/jexl3/internal/introspection/SandboxUberspect.java @@ -96,7 +96,16 @@ public final class SandboxUberspect implements JexlUberspect { @Override public Iterator<?> getIterator(final Object obj) { - return uberspect.getIterator(obj); + if (obj != null) { + // route iteration through the sandbox: consult the "iterator" method permission for the + // object's class so an explicitly restricted class cannot be iterated over (f006). + final Class<?> clazz = obj instanceof Class<?> ? (Class<?>) obj : obj.getClass(); + final String actual = sandbox.execute(clazz, "iterator"); + if (actual != null && actual != JexlSandbox.NULL) { + return uberspect.getIterator(obj); + } + } + return null; } @Override diff --git a/src/main/java/org/apache/commons/jexl3/introspection/JexlPermissions.java b/src/main/java/org/apache/commons/jexl3/introspection/JexlPermissions.java index a59590f3..4722a63a 100644 --- a/src/main/java/org/apache/commons/jexl3/introspection/JexlPermissions.java +++ b/src/main/java/org/apache/commons/jexl3/introspection/JexlPermissions.java @@ -41,12 +41,28 @@ import org.apache.commons.logging.LogFactory; * access a constructor, method or field before exposition to the {@link JexlUberspect}. The restrictions * are applied in all cases, for any {@link org.apache.commons.jexl3.introspection.JexlUberspect.ResolverStrategy}. * </p> + * <p><strong>Arithmetic exemptions.</strong> Permissions gate <em>reflective</em> access performed through the + * {@link JexlUberspect}. They do not gate the handful of {@link java.lang.Object} / collection methods that + * {@link org.apache.commons.jexl3.JexlArithmetic} invokes directly (without reflection) as operator fast-paths - + * namely {@code equals} and {@code compareTo} (comparison operators), {@code toString} (string coercion and + * concatenation), and {@code isEmpty}/{@code size}/{@code contains} (the {@code empty}, {@code size} and + * {@code =~} operators on collections and maps). These are language-level operations analogous to arithmetic on + * numbers and are always available; a permission entry denying, say, {@code equals} does not disable the + * {@code ==} operator. To constrain those, derive {@link org.apache.commons.jexl3.JexlArithmetic}.</p> * <p><strong>Security disclaimer.</strong> Neither {@link #RESTRICTED} nor {@link #SECURE} is exhaustive, and neither * must be considered completely safe or sufficient on its own for executing untrusted user input. They are hardened * baselines, not guarantees. Any application that evaluates untrusted scripts <em>must</em> define its own tailored, * strict whitelist of exactly the classes, methods and fields its scripts legitimately need - ideally by composing on * top of {@link #NONE} (which denies everything) via {@link #create(String...)} / {@link #compose(String...)} - and * audit the result with {@link #logging()}.</p> + * <p><strong>Compiler surface.</strong> Permissions gate reflective access; they do not gate JEXL's own compiler. + * {@link #RESTRICTED} denies the second-stage compiler surface reachable through reflection - {@code JexlBuilder}, + * and the {@code createScript}/{@code createExpression}/{@code createJxltEngine} methods on + * {@link org.apache.commons.jexl3.JexlEngine} as well as {@code createExpression}/{@code createTemplate} on + * {@link org.apache.commons.jexl3.JxltEngine} - so a script that gets hold of a live engine cannot compile and run + * further scripts. Note, however, that a {@link org.apache.commons.jexl3.JexlScript} value passed into a script and + * invoked as a lambda (e.g. {@code fn(args)}) executes <em>by design</em> without a reflective call and is therefore + * not mediated by these permissions; only pass already-compiled scripts a caller trusts.</p> * <p>This complements using a dedicated {@link ClassLoader} and/or {@link SecurityManager} - being deprecated - * and possibly {@link JexlSandbox} with a simpler mechanism. The {@link org.apache.commons.jexl3.annotations.NoJexl} * annotation processing is actually performed using the result of calling {@link #parse(String...)} with no arguments; @@ -526,7 +542,11 @@ public interface JexlPermissions { "java.io -{ +PrintWriter{ -PrintWriter(); } +Writer{} +StringWriter{} +Reader{} +InputStream{} +OutputStream{} }", "java.nio +{ -ByteBuffer { allocateDirect(); } }", "java.nio.charset +{}", - "org.apache.commons.jexl3 +{ -JexlBuilder{} -JexlConfigLoader{} }" + // deny the second-stage compiler surface: a script that gets hold of a live engine (via the + // context or the thread-local) must not be able to compile and run further scripts (f009/f036) + "org.apache.commons.jexl3 +{ -JexlBuilder{} -JexlConfigLoader{}" + + " -JexlEngine { getThreadEngine(); setThreadContext(); createExpression(); createScript(); createJxltEngine(); }" + + " -JxltEngine { createExpression(); createTemplate(); } }" ); /** diff --git a/src/test/java/org/apache/commons/jexl3/internal/introspection/PermissionsTest.java b/src/test/java/org/apache/commons/jexl3/internal/introspection/PermissionsTest.java index 3409a412..9f585386 100644 --- a/src/test/java/org/apache/commons/jexl3/internal/introspection/PermissionsTest.java +++ b/src/test/java/org/apache/commons/jexl3/internal/introspection/PermissionsTest.java @@ -587,7 +587,10 @@ class PermissionsTest { "java.lang {{ Runtime {} }", "java.rmi {}}", "java.io { Text File {} }", - "java.io { File { m.x } }" + "java.io { File { m.x } }", + // f008: mixing +/- signs on a single element is ambiguous and rejected + "java.io { File { + -m(); } }", + "java.io { File { - +f; } }" }; // @formatter:on for (final String src : srcs) { @@ -595,6 +598,43 @@ class PermissionsTest { } } + @Test + void testInnerClassDenySign() { + // f008: a '-' sign on an inner class must deny it, even inside an allowing (+) outer class; + // previously the inner-class sign was discarded and the outer polarity leaked in + final String pkg = "org.apache.commons.jexl3.internal.introspection"; + final String src = pkg + " { PermissionsTest { +Outer { -Inner {} } } }"; + final Permissions p = (Permissions) JexlPermissions.parse(src); + final Method callMeNot = getMethod(Outer.Inner.class, "callMeNot"); + assertFalse(p.allow(callMeNot), "-Inner must be denied"); + } + + @Test + void testPositiveMemberDoesNotLeakToInnerClass() { + // f008: a '+' before a member must not flip the enclosing class polarity and leak into a + // following inner class; Inner inherits Outer's deny polarity and stays denied + final String pkg = "org.apache.commons.jexl3.internal.introspection"; + final String src = pkg + " { PermissionsTest { Outer { +hashCode(); Inner {} } } }"; + final Permissions p = (Permissions) JexlPermissions.parse(src); + final Method callMeNot = getMethod(Outer.Inner.class, "callMeNot"); + assertFalse(p.allow(callMeNot), "Inner must stay denied despite a preceding +member"); + } + + @Test + void testEngineCompilerDeniedUnderRestricted() { + // f009/f036: a live engine reachable from a script must not act as a second-stage compiler + assertFalse(RESTRICTED.allow(getMethod(JexlEngine.class, "createScript"))); + assertFalse(RESTRICTED.allow(getMethod(JexlEngine.class, "createExpression"))); + assertFalse(RESTRICTED.allow(getMethod(JexlEngine.class, "createJxltEngine"))); + assertFalse(RESTRICTED.allow(getMethod(JexlEngine.class, "getThreadEngine"))); + + final JexlEngine jexl = new JexlBuilder().permissions(RESTRICTED).safe(false).strict(true).create(); + final JexlContext ctxt = new MapContext(); + ctxt.set("e", jexl); + final JexlScript script = jexl.createScript("e.createScript('1 + 1')"); + assertThrows(JexlException.Method.class, () -> script.execute(ctxt)); + } + @Test void testPermissions0() throws Exception { runTestPermissions(permissions0()); diff --git a/src/test/java/org/apache/commons/jexl3/introspection/SandboxTest.java b/src/test/java/org/apache/commons/jexl3/introspection/SandboxTest.java index b5e1c129..d3994072 100644 --- a/src/test/java/org/apache/commons/jexl3/introspection/SandboxTest.java +++ b/src/test/java/org/apache/commons/jexl3/introspection/SandboxTest.java @@ -24,6 +24,7 @@ import static org.junit.jupiter.api.Assertions.fail; import java.util.ArrayList; import java.util.Arrays; +import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -206,12 +207,39 @@ class SandboxTest extends JexlTestCase { void tryMeARiver(); } + /** A simple iterable used to verify sandboxed iteration (f006). */ + public static class Iterable386 implements Iterable<Integer> { + private final List<Integer> values = Arrays.asList(1, 2, 3); + @Override + public Iterator<Integer> iterator() { + return values.iterator(); + } + } + static final Log LOGGER = LogFactory.getLog(SandboxTest.class.getName()); public SandboxTest() { super("SandboxTest"); } + @Test + void testIteratorBlock() { + // f006: the sandbox must gate iteration; blocking the "iterator" method denies foreach + final String src = "var sum = 0; for (var i : it) { sum += i; } sum"; + final Iterable386 it = new Iterable386(); + + // allow-by-default sandbox: iteration works and sums 1 + 2 + 3 + final JexlSandbox open = new JexlSandbox(); + final JexlEngine ojexl = new JexlBuilder().sandbox(open).strict(true).safe(false).create(); + assertEquals(6, ojexl.createScript(src, "it").execute(null, it)); + + // block the iterator method: getIterator returns null and the loop body never runs + final JexlSandbox sandbox = new JexlSandbox(); + sandbox.block(Iterable386.class.getName()).execute("iterator"); + final JexlEngine sjexl = new JexlBuilder().sandbox(sandbox).strict(true).safe(false).create(); + assertEquals(0, sjexl.createScript(src, "it").execute(null, it)); + } + @Test void testCantSeeMe() { final JexlContext jc = new MapContext();
