[prev in list] [next in list] [prev in thread] [next in thread]
List: jakarta-commons-dev
Subject: [commons-jexl] 01/07: JEXL-381: change permissions default, update tests, add javadoc;
From: henrib () apache ! org
Date: 2022-10-31 14:25:53
Message-ID: 20221031142552.75B1C440184 () gitbox2-he-fi ! apache ! org
[Download RAW message or body]
This is an automated email from the ASF dual-hosted git repository.
henrib pushed a commit to branch JEXL-381
in repository https://gitbox.apache.org/repos/asf/commons-jexl.git
commit eb4f860d74fa115cda013785b37fbccf6d49deb9
Author: henrib <henrib@apache.org>
AuthorDate: Fri Oct 21 19:33:48 2022 +0200
JEXL-381: change permissions default, update tests, add javadoc;
---
.../java/org/apache/commons/jexl3/JexlBuilder.java | 62 ++++++++++++++-----
.../org/apache/commons/jexl3/internal/Engine.java | 4 +-
.../jexl3/internal/introspection/Introspector.java | 4 +-
.../jexl3/internal/introspection/Permissions.java | 4 +-
.../jexl3/internal/introspection/Uberspect.java | 2 +-
.../jexl3/introspection/JexlPermissions.java | 70 +++++++++++++++++++++-
.../org/apache/commons/jexl3/Issues300Test.java | 59 ++++++++++++++++++
.../apache/commons/jexl3/PropertyAccessTest.java | 3 +-
.../jexl3/internal/introspection/NoJexlTest.java | 7 +--
.../commons/jexl3/introspection/SandboxTest.java | 7 ++-
.../jexl3/scripting/JexlScriptEngineTest.java | 15 +++--
11 files changed, 201 insertions(+), 36 deletions(-)
diff --git a/src/main/java/org/apache/commons/jexl3/JexlBuilder.java \
b/src/main/java/org/apache/commons/jexl3/JexlBuilder.java index f0001000..52316bf2 \
100644
--- a/src/main/java/org/apache/commons/jexl3/JexlBuilder.java
+++ b/src/main/java/org/apache/commons/jexl3/JexlBuilder.java
@@ -31,25 +31,31 @@ import java.nio.charset.Charset;
/**
* Configures and builds a JexlEngine.
*
- * <p>The builder allow fine-tuning an engine instance behavior according to various \
control needs.</p>
- *
- * <p>Broad configurations elements are controlled through the features ({@link \
JexlFeatures}) that can restrict JEXL + * <p>
+ * The builder allow fine-tuning an engine instance behavior according to \
various control needs. + * Check <em>{@link #JexlBuilder()}</em> for permission \
impacts starting with <em>JEXL 3.3</em>. + * </p><p>
+ * Broad configurations elements are controlled through the features ({@link \
JexlFeatures}) that can restrict JEXL
* syntax - for instance, only expressions with no-side effects - and permissions \
({@link JexlPermissions}) that control
- * the visible set of objects - for instance, avoiding access to any object in \
java.rmi.* -. </p>
- *
- * <p>Fine error control and runtime-overridable behaviors are implemented through \
options ({@link JexlOptions}). Most + * the visible set of objects - for instance, \
avoiding access to any object in java.rmi.* -. + * </p><p>
+ * Fine error control and runtime-overridable behaviors are implemented through \
options ({@link JexlOptions}). Most
* common flags accessible from the builder are reflected in its options ({@link \
#options()}).
- * <p>The <code>silent</code> flag tells the engine what to do with the error; when \
true, errors are logged as
- * warning, when false, they throw {@link JexlException} exceptions.</p>
- * <p>The <code>strict</code> flag tells the engine when and if null as operand is \
considered an error. The <code>safe</code> + * </p><p>
+ * The <code>silent</code> flag tells the engine what to do with the error; when \
true, errors are logged as + * warning, when false, they throw {@link JexlException} \
exceptions. + * </p><p>
+ * The <code>strict</code> flag tells the engine when and if null as operand is \
considered an error. The <code>safe</code>
* flog determines if safe-navigation is used. Safe-navigation allows an evaluation \
shortcut and return null in expressions
- * that attempts dereferencing null, typically a method call or accessing a \
property.</p>
- * <p>The <code>lexical</code> and <code>lexicalShade</code> flags can be used to \
enforce a lexical scope for + * that attempts dereferencing null, typically a method \
call or accessing a property. + * </p><p>
+ * The <code>lexical</code> and <code>lexicalShade</code> flags can be used to \
enforce a lexical scope for
* variables and parameters. The <code>lexicalShade</code> can be used to further \
ensure no global variable can be
* used with the same name as a local one even after it goes out of scope. The \
corresponding feature flags should be
- * preferred since they will detect violations at parsing time. (see {@link \
JexlFeatures})</p>
- *
- * <p>The following rules apply on silent and strict flags:</p>
+ * preferred since they will detect violations at parsing time. (see {@link \
JexlFeatures}) + * </p><p>
+ * The following rules apply on silent and strict flags:
+ * </p>
* <ul>
* <li>When "silent" & "not-strict":
* <p> 0 & null should be indicators of "default" values so that even in an case \
of error, @@ -86,7 +92,7 @@ public class JexlBuilder {
private JexlUberspect.ResolverStrategy strategy = null;
/** The set of permissions. */
- private JexlPermissions permissions = null;
+ private JexlPermissions permissions = JexlPermissions.RESTRICTED;
/** The sandbox. */
private JexlSandbox sandbox = null;
@@ -127,6 +133,32 @@ public class JexlBuilder {
/** The features. */
private JexlFeatures features = null;
+ /**
+ * Default constructor.
+ * <p>
+ * As of JEXL 3.3, to reduce the security risks inherent to JEXL"s purpose, \
the builder will use a set of + * restricted permissions as a default to create \
the JexlEngine instance. This will greatly reduce which classes + * and methods \
are visible to JEXL and usable in scripts using default implicit behaviors. + * \
</p><p> + * However, without mitigation, this change will likely break some \
scripts at runtime, especially those exposing + * your own class instances \
through arguments, contexts or namespaces. + * The new default set of allowed \
packages and denied classes is described by {@link JexlPermissions#RESTRICTED}. + \
* </p><p> + * The recommended mitigation if your usage of JEXL is impacted is to \
first thoroughly review what should be + * allowed and exposed to script authors \
and implement those through a set of {@link JexlPermissions}; + * those are \
easily created using {@link JexlPermissions#parse(String...)}. + * </p><p>
+ * In the urgent case of a strict 3.2 compatiblity, the simplest and fastest \
mitigation is to use the 'unrestricted' + * set of permissions. The builder must \
be explicit about it using + * <code>new JexlBuilder().permissions({@link \
JexlPermissions#UNRESTRICTED})</code>. + * </p><p>
+ * Note that an explicit call to {@link #uberspect(JexlUberspect)} will \
supersede this behavior using the + * {@link JexlUberspect} provided instance in \
the {@link JexlEngine}. + * </p>
+ * @since 3.3
+ */
+ public JexlBuilder() {}
+
/**
* Sets the JexlUberspect instance the engine will use.
*
diff --git a/src/main/java/org/apache/commons/jexl3/internal/Engine.java \
b/src/main/java/org/apache/commons/jexl3/internal/Engine.java index \
e44802ab..9d72bd8b 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/Engine.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/Engine.java
@@ -248,7 +248,7 @@ public class Engine extends JexlEngine {
* <p>This is lazily initialized to avoid building a default instance if there
* is no use for it. The main reason for not using the default Uberspect \
instance is to
* be able to use a (low level) introspector created with a given logger
- * instead of the default one.</p>
+ * instead of the default one and even more so for with a different (restricted) \
set of permissions.</p>
* @param logger the logger to use for the underlying Uberspect
* @param strategy the property resolver strategy
* @param permissions the introspection permissions
@@ -261,7 +261,7 @@ public class Engine extends JexlEngine {
final JexlPermissions permissions) {
if ((logger == null || logger.equals(LogFactory.getLog(JexlEngine.class)))
&& (strategy == null || strategy == JexlUberspect.JEXL_STRATEGY)
- && permissions == null || permissions == Permissions.DEFAULT) {
+ && (permissions == null || permissions == JexlPermissions.UNRESTRICTED)) \
{ return UberspectHolder.UBERSPECT;
}
return new Uberspect(logger, strategy, permissions);
diff --git a/src/main/java/org/apache/commons/jexl3/internal/introspection/Introspector.java \
b/src/main/java/org/apache/commons/jexl3/internal/introspection/Introspector.java \
index ba7a5bc5..869c5405 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/introspection/Introspector.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/introspection/Introspector.java
@@ -94,7 +94,7 @@ public final class Introspector {
* @param cloader the class loader
*/
public Introspector(final Log log, final ClassLoader cloader) {
- this(log, cloader, Permissions.DEFAULT);
+ this(log, cloader, null);
}
/**
@@ -106,7 +106,7 @@ public final class Introspector {
public Introspector(final Log log, final ClassLoader cloader, final \
JexlPermissions perms) { this.logger = log;
this.loader = cloader;
- this.permissions = perms;
+ this.permissions = perms == null? Permissions.RESTRICTED : perms;
}
/**
diff --git a/src/main/java/org/apache/commons/jexl3/internal/introspection/Permissions.java \
b/src/main/java/org/apache/commons/jexl3/internal/introspection/Permissions.java \
index 177cf7a4..02788748 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/introspection/Permissions.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/introspection/Permissions.java
@@ -225,9 +225,9 @@ public class Permissions implements JexlPermissions {
}
/**
- * The default singleton.
+ * The no-restriction introspection permission singleton.
*/
- public static final Permissions DEFAULT = new Permissions();
+ public static final Permissions UNRESTRICTED = new Permissions();
/**
* @return the packages
diff --git a/src/main/java/org/apache/commons/jexl3/internal/introspection/Uberspect.java \
b/src/main/java/org/apache/commons/jexl3/internal/introspection/Uberspect.java index \
8e8c3e37..1e2a01c8 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/introspection/Uberspect.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/introspection/Uberspect.java
@@ -92,7 +92,7 @@ public class Uberspect implements JexlUberspect {
public Uberspect(final Log runtimeLogger, final JexlUberspect.ResolverStrategy \
sty, final JexlPermissions perms) {
logger = runtimeLogger == null? LogFactory.getLog(JexlEngine.class) : \
runtimeLogger; strategy = sty == null? JexlUberspect.JEXL_STRATEGY : sty;
- permissions = perms == null? Permissions.DEFAULT : perms;
+ permissions = perms == null? Permissions.RESTRICTED : perms;
ref = new SoftReference<Introspector>(null);
loader = new SoftReference<ClassLoader>(getClass().getClassLoader());
operatorMap = new ConcurrentHashMap<Class<? extends JexlArithmetic>, \
Set<JexlOperator>>();
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 \
f0cad14d..2fca219c 100644
--- a/src/main/java/org/apache/commons/jexl3/introspection/JexlPermissions.java
+++ b/src/main/java/org/apache/commons/jexl3/introspection/JexlPermissions.java
@@ -46,6 +46,7 @@ import java.lang.reflect.Method;
* @since 3.3
*/
public interface JexlPermissions {
+
/**
* Checks whether a package allows JEXL introspection.
* <p>If the package disallows JEXL introspection, none of its classes or \
interfaces are visible @@ -165,7 +166,74 @@ public interface JexlPermissions {
* @since 3.3
*/
static JexlPermissions parse(String... src) {
- return src == null || src.length == 0? Permissions.DEFAULT : new \
PermissionsParser().parse(src); + return src == null || src.length == 0? \
Permissions.UNRESTRICTED : new PermissionsParser().parse(src); }
+ /**
+ * The unrestricted permissions.
+ * <p>This enables any public class, method, constructor or field to be visible \
to JEXL and used in scripts.</p> + * @since 3.3
+ */
+ public static final JexlPermissions UNRESTRICTED = Permissions.UNRESTRICTED;
+ /**
+ * A restricted singleton.
+ * <p>The RESTRICTED set is built using the following allowed packages and \
denied packages/classes.</p> + * <p>Of particular importance are the restrictions \
on the {@link System}, + * {@link Runtime}, {@link ProcessBuilder}, {@link Class} \
and those on {@link java.net}, {@link java.net}, + * {@link java.io} and {@link \
java.lang.reflect} that should provide a decent level of isolation between the \
scripts + * and its host.
+ * </p>
+ * <p>
+ * As a simple guide, any line that ends with ".*" is allowing a \
package, any other is + * denying a package, class or method.
+ * </p>
+ * <ul>
+ * <li>java.nio.*</li>
+ * <li>java.io.*</li>
+ * <li>java.lang.*</li>
+ * <li>java.math.*</li>
+ * <li>java.text.*</li>
+ * <li>java.util.*</li>
+ * <li>org.w3c.dom.*</li>
+ * <li>org.apache.commons.jexl3.*</li>
+ *
+ * <li>org.apache.commons.jexl3 { JexlBuilder {} }</li>
+ * <li>org.apache.commons.jexl3.internal { Engine {} }</li>
+ * <li>java.lang { Runtime {} System {} ProcessBuilder {} Class {} }</li>
+ * <li>java.lang.annotation {}</li>
+ * <li>java.lang.instrument {}</li>
+ * <li>java.lang.invoke {}</li>
+ * <li>java.lang.management {}</li>
+ * <li>java.lang.ref {}</li>
+ * <li>java.lang.reflect {}</li>
+ * <li>java.net {}</li>
+ * <li>java.io { File { } }</li>
+ * <li>java.nio { Path { } Paths { } Files { } }</li>
+ * <li>java.rmi {}</li>
+ * </ul>
+ */
+ public static final JexlPermissions RESTRICTED = JexlPermissions.parse(
+ "# Restricted Uberspect Permissions",
+ "java.nio.*",
+ "java.io.*",
+ "java.lang.*",
+ "java.math.*",
+ "java.text.*",
+ "java.util.*",
+ "org.w3c.dom.*",
+ "org.apache.commons.jexl3.*",
+ "org.apache.commons.jexl3 { JexlBuilder {} }",
+ "org.apache.commons.jexl3.internal { Engine {} }",
+ "java.lang { Runtime {} System {} ProcessBuilder {} Class {} }",
+ "java.lang.annotation {}",
+ "java.lang.instrument {}",
+ "java.lang.invoke {}",
+ "java.lang.management {}",
+ "java.lang.ref {}",
+ "java.lang.reflect {}",
+ "java.net {}",
+ "java.io { File { } }",
+ "java.nio { Path { } Paths { } Files { } }",
+ "java.rmi"
+ );
}
diff --git a/src/test/java/org/apache/commons/jexl3/Issues300Test.java \
b/src/test/java/org/apache/commons/jexl3/Issues300Test.java index eec25e90..78a799e7 \
100644
--- a/src/test/java/org/apache/commons/jexl3/Issues300Test.java
+++ b/src/test/java/org/apache/commons/jexl3/Issues300Test.java
@@ -31,6 +31,8 @@ import java.util.HashMap;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
import static org.junit.Assert.assertEquals;
@@ -951,4 +953,61 @@ public class Issues300Test {
Assert.assertEquals("'\\b\\t\\f'", parsed);
}
+ /**
+ * Mock driver.
+ */
+ public static class Driver0930 {
+ private String name;
+ Driver0930(String n) {
+ name = n;
+ }
+ public String getAttributeName() {
+ return name;
+ }
+ }
+
+ public static class Context0930 extends MapContext {
+ /**
+ * This allows using a JEXL lmabda as a filter.
+ * @param stream the stream
+ * @param filter the lambda to use as filter
+ * @return the filtered stream
+ */
+ public Stream<?> filter(Stream<?> stream, JexlScript filter) {
+ return stream.filter(x -> Boolean.TRUE.equals(filter.execute(this, x)));
+ }
+ }
+
+ @Test
+ public void testStackOvflw20220930() {
+ // fill some drivers in a list
+ List<Driver0930> values = new ArrayList<>();
+ for(int i = 0; i < 8; ++i) {
+ values.add(new Driver0930("drvr" + Integer.toOctalString(i)));
+ }
+ for(int i = 0; i < 4; ++i) {
+ values.add(new Driver0930("favorite" + Integer.toOctalString(i)));
+ }
+ // Use a context that can filter and that exposes Collectors
+ JexlEngine jexl = new JexlBuilder().safe(false).create();
+ JexlContext context = new Context0930();
+ context.set("values", values);
+ context.set("Collectors", Collectors.class);
+ // The script with a JEXL 3.2 (lambda function) and 3.3 syntax (lambda \
expression) + String src32 = "values.stream().filter((driver) ->{ \
driver.attributeName =^ 'favorite' }).collect(Collectors.toList())"; + String \
src33 = "values.stream().filter(driver -> driver.attributeName =^ \
'favorite').collect(Collectors.toList())"; + for(String src : \
Arrays.asList(src32, src33)) { + JexlExpression s = \
jexl.createExpression(src); + Assert.assertNotNull(s);
+
+ Object r = s.evaluate(context);
+ Assert.assertNotNull(r);
+ // got a filtered list of 4 drivers whose attribute name starts with \
'favorite' + List<Driver0930> l = (List<Driver0930>) r;
+ Assert.assertEquals(4, l.size());
+ for (Driver0930 d : l) {
+ Assert.assertTrue(d.getAttributeName().startsWith("favorite"));
+ }
+ }
+ }
}
diff --git a/src/test/java/org/apache/commons/jexl3/PropertyAccessTest.java \
b/src/test/java/org/apache/commons/jexl3/PropertyAccessTest.java index \
7883c0a6..b250fc36 100644
--- a/src/test/java/org/apache/commons/jexl3/PropertyAccessTest.java
+++ b/src/test/java/org/apache/commons/jexl3/PropertyAccessTest.java
@@ -23,6 +23,7 @@ import org.apache.commons.jexl3.internal.Debugger;
import org.apache.commons.jexl3.internal.introspection.IndexedType;
import org.apache.commons.jexl3.internal.introspection.Permissions;
import org.apache.commons.jexl3.internal.introspection.Uberspect;
+import org.apache.commons.jexl3.introspection.JexlPermissions;
import org.apache.commons.jexl3.introspection.JexlUberspect;
import org.apache.commons.jexl3.junit.Asserter;
import org.apache.commons.logging.LogFactory;
@@ -384,7 +385,7 @@ public class PropertyAccessTest extends JexlTestCase {
x.put(2, "123456789");
ctx.set("x", x);
final JexlEngine engine = new JexlBuilder()
- .uberspect(new Uberspect(null, null, null))
+ .uberspect(new Uberspect(null, null, JexlPermissions.UNRESTRICTED))
.strict(true).silent(false).create();
String stmt = "x.2.class.name";
JexlScript script = engine.createScript(stmt);
diff --git a/src/test/java/org/apache/commons/jexl3/internal/introspection/NoJexlTest.java \
b/src/test/java/org/apache/commons/jexl3/internal/introspection/NoJexlTest.java index \
acfb87c5..46c29318 100644
--- a/src/test/java/org/apache/commons/jexl3/internal/introspection/NoJexlTest.java
+++ b/src/test/java/org/apache/commons/jexl3/internal/introspection/NoJexlTest.java
@@ -17,17 +17,12 @@
package org.apache.commons.jexl3.internal.introspection;
import org.apache.commons.jexl3.annotations.NoJexl;
-import org.apache.commons.jexl3.introspection.JexlPermissions;
import org.junit.Assert;
import org.junit.Test;
import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
-import java.util.Arrays;
-import java.util.HashSet;
-import java.util.Map;
-import java.util.Set;
/**
* Checks the CacheMap.MethodKey implementation
@@ -88,7 +83,7 @@ public class NoJexlTest {
@Test
public void testNoJexlPermissions() throws Exception {
- Permissions p = Permissions.DEFAULT;
+ Permissions p = Permissions.UNRESTRICTED;
Assert.assertFalse(p.allow((Field) null));
Assert.assertFalse(p.allow((Package) null));
Assert.assertFalse(p.allow((Method) null));
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 \
26342818..0a3fda12 100644
--- a/src/test/java/org/apache/commons/jexl3/introspection/SandboxTest.java
+++ b/src/test/java/org/apache/commons/jexl3/introspection/SandboxTest.java
@@ -349,7 +349,12 @@ public class SandboxTest extends JexlTestCase {
// can not create a new file
sandbox.block(java.io.File.class.getName()).execute("");
- final JexlEngine sjexl = new \
JexlBuilder().sandbox(sandbox).safe(false).strict(true).create(); + final \
JexlEngine sjexl = new JexlBuilder() + \
.permissions(JexlPermissions.UNRESTRICTED) + .sandbox(sandbox)
+ .safe(false)
+ .strict(true)
+ .create();
String expr;
JexlScript script;
diff --git a/src/test/java/org/apache/commons/jexl3/scripting/JexlScriptEngineTest.java \
b/src/test/java/org/apache/commons/jexl3/scripting/JexlScriptEngineTest.java index \
024da76f..91e5d78b 100644
--- a/src/test/java/org/apache/commons/jexl3/scripting/JexlScriptEngineTest.java
+++ b/src/test/java/org/apache/commons/jexl3/scripting/JexlScriptEngineTest.java
@@ -29,6 +29,7 @@ import javax.script.ScriptEngine;
import javax.script.ScriptEngineManager;
import javax.script.ScriptException;
+import org.apache.commons.jexl3.JexlException;
import org.junit.Assert;
import org.junit.Test;
@@ -83,12 +84,16 @@ public class JexlScriptEngineTest {
final Integer initialValue = 123;
Assert.assertEquals(initialValue,engine.eval("123"));
Assert.assertEquals(initialValue,engine.eval("0;123"));// multiple \
statements
- final long time1 = System.currentTimeMillis();
- final Long time2 = (Long) engine.eval(
- "sys=context.class.forName(\"java.lang.System\");"
- +"now=sys.currentTimeMillis();"
+ try {
+ final Long time2 = (Long) engine.eval(
+ "sys=context.class.forName(\"java.lang.System\");"
+ + "now=sys.currentTimeMillis();"
);
- Assert.assertTrue("Must take some time to process this",time1 <= time2);
+ Assert.fail("default engine no longer accesses System classes");
+ } catch(ScriptException xscript) {
+ JexlException.Method xjexl = (JexlException.Method) xscript.getCause();
+ Assert.assertEquals("forName", xjexl.getMethod());
+ }
engine.put("value", initialValue);
Assert.assertEquals(initialValue,engine.get("value"));
final Integer newValue = 124;
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic