This is an automated email from the ASF dual-hosted git repository.
henrib pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-jexl.git
The following commit(s) were added to refs/heads/master by this push:
new 88c18aea JEXL-457: updating RESTRICTED to deny all java.lang
sub-packages; - tighten permission computation so that explicit allow must be
present; - ensure class constructor is explicitly allowed;
88c18aea is described below
commit 88c18aeafeeda0f0209ca57f995a4db9414f990e
Author: Henrib <[email protected]>
AuthorDate: Fri Mar 6 08:46:42 2026 +0100
JEXL-457: updating RESTRICTED to deny all java.lang sub-packages;
- tighten permission computation so that explicit allow must be present;
- ensure class constructor is explicitly allowed;
---
.../jexl3/internal/introspection/Permissions.java | 34 ++++++-
.../jexl3/introspection/JexlPermissions.java | 34 ++++---
.../commons/jexl3/scripting/JexlScriptEngine.java | 3 +-
.../org/apache/commons/jexl3/ArithmeticTest.java | 5 +-
.../org/apache/commons/jexl3/Issues400Test.java | 1 +
.../java/org/apache/commons/jexl3/SwitchTest.java | 7 ++
.../internal/introspection/PermissionsTest.java | 101 ++++++++++++++++-----
7 files changed, 140 insertions(+), 45 deletions(-)
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 5a362600..6c2bdfa6 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
@@ -28,6 +28,7 @@ import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.BiPredicate;
import org.apache.commons.jexl3.annotations.NoJexl;
import org.apache.commons.jexl3.introspection.JexlPermissions;
@@ -320,7 +321,7 @@ public class Permissions implements JexlPermissions {
}
// explicit |= ...
if (!explicit[0]) {
- explicit[0] = wildcardAllow(clazz);
+ explicit[0] = wildcardAllow(clazz) || specifiedAllow(clazz,
override, (njc, m) -> !njc.deny(m));
}
return true;
} catch (final NoSuchMethodException ex) {
@@ -357,6 +358,31 @@ public class Permissions implements JexlPermissions {
return wildcardAllow(clazz);
}
+ /**
+ * Determines whether a specified permission check is allowed for a given
class.
+ * The check involves verifying if a class or its corresponding package
explicitly permits
+ * a name (e.g., method) based on a given condition.
+ *
+ * @param <T> the type of the name to check (e.g., method, constructor)
+ * @param clazz the class to evaluate (not null)
+ * @param name the name to verify (not null)
+ * @param check the condition to test whether the specified name is
allowed (not null)
+ * @return true if the specified name is allowed based on the condition,
false otherwise
+ */
+ private <T> boolean specifiedAllow(final Class<?> clazz, T name,
BiPredicate<NoJexlClass, T> check) {
+ final NoJexlPackage njp =
packages.get(ClassTool.getPackageName(clazz));
+ if (njp != null && check != null) {
+ // there is a package permission, check if there is a class
permission
+ final NoJexlClass njc = njp.getNoJexl(clazz);
+ // if there is a class permission, perform the check
+ if (njc != null) {
+ return check.test(njc, name);
+ }
+ }
+ // nothing explicit
+ return false;
+ }
+
/**
* Checks whether a field explicitly disallows JEXL introspection.
*
@@ -379,7 +405,7 @@ public class Permissions implements JexlPermissions {
return false;
}
// check wildcards
- return wildcardAllow(clazz);
+ return wildcardAllow(clazz) || specifiedAllow(clazz, field, (njc, m)
-> !njc.deny(m));
}
/**
@@ -401,8 +427,8 @@ public class Permissions implements JexlPermissions {
return false;
}
Class<?> clazz = method.getDeclaringClass();
- // gather if any implementation of the method is explicitly allowed by
the packages
- final boolean[] explicit = { wildcardAllow(clazz) };
+ // gather if the packages explicitly allow any implementation of the
method
+ final boolean[] explicit = { wildcardAllow(clazz) ||
specifiedAllow(clazz, method, (njc, m) -> !njc.deny(m)) };
// let's walk all interfaces
for (final Class<?> inter : clazz.getInterfaces()) {
if (!allow(inter, method, explicit)) {
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 d84817b5..304ab139 100644
--- a/src/main/java/org/apache/commons/jexl3/introspection/JexlPermissions.java
+++ b/src/main/java/org/apache/commons/jexl3/introspection/JexlPermissions.java
@@ -188,7 +188,7 @@ public interface JexlPermissions {
* 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 Runtime}, {@link ProcessBuilder}, {@link Class} and those on
{@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>
@@ -198,7 +198,6 @@ public interface JexlPermissions {
* </p>
* <ul>
* <li>java.nio.*</li>
- * <li>java.io.*</li>
* <li>java.lang.*</li>
* <li>java.math.*</li>
* <li>java.text.*</li>
@@ -207,46 +206,55 @@ public interface JexlPermissions {
* <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>org.apache.commons.jexl3.introspection { JexlPermissions {}
JexlPermissions$ClassPermissions {} }</li>
+ * <li>org.apache.commons.jexl3.internal { Engine {} Engine32 {}
TemplateEngine {} }</li>
+ * <li>org.apache.commons.jexl3.internal.introspection { Uberspect {}
Introspector {} }</li>
+ * <li>java.lang { Runtime {} System {} ProcessBuilder {} Process {}
RuntimePermission {} SecurityManager {} Thread {} ThreadGroup {} Class {} }</li>
* <li>java.lang.annotation {}</li>
+ * <li>java.lang.classfile {}</li>
+ * <li>java.lang.constant {}</li>
+ * <li>java.lang.foreign {}</li>
* <li>java.lang.instrument {}</li>
* <li>java.lang.invoke {}</li>
* <li>java.lang.management {}</li>
+ * <li>java.lang.module {}</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.io { +PrintWriter {} +Writer {} +StringWriter {} +Reader {}
+InputStream {} +OutputStream {} }</li>
+ * <li>java.nio { Path {} Paths {} Files {} }</li>
* <li>java.rmi {}</li>
* </ul>
*/
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.introspection { JexlPermissions {}
JexlPermissions$ClassPermissions {} }",
- "org.apache.commons.jexl3.internal { Engine {} Engine32 {}
TemplateEngine {} }",
- "org.apache.commons.jexl3.internal.introspection { Uberspect {}
Introspector {} }",
+ "org.apache.commons.jexl3 { JexlBuilder{} }",
+ "org.apache.commons.jexl3.introspection { JexlPermissions{}
JexlPermissions$ClassPermissions{} }",
+ "org.apache.commons.jexl3.internal { Engine{} Engine32{}
TemplateEngine{} }",
+ "org.apache.commons.jexl3.internal.introspection { Uberspect{}
Introspector{} }",
"java.lang { Runtime{} System{} ProcessBuilder{} Process{}" +
" RuntimePermission{} SecurityManager{}" +
" Thread{} ThreadGroup{} Class{} }",
"java.lang.annotation {}",
+ "java.lang.classfile {}",
+ "java.lang.constant {}",
+ "java.lang.foreign {}",
"java.lang.instrument {}",
"java.lang.invoke {}",
"java.lang.management {}",
+ "java.lang.module {}",
"java.lang.ref {}",
"java.lang.reflect {}",
"java.net {}",
- "java.io { File{} FileDescriptor{} }",
- "java.nio { Path { } Paths { } Files { } }",
+ "java.io { +PrintWriter{} +Writer {} +StringWriter {} +Reader {}
+InputStream {} +OutputStream {} }",
+ "java.nio { Path {} Paths {} Files {} }",
"java.rmi"
);
diff --git
a/src/main/java/org/apache/commons/jexl3/scripting/JexlScriptEngine.java
b/src/main/java/org/apache/commons/jexl3/scripting/JexlScriptEngine.java
index 5b6c9068..52c9db7e 100644
--- a/src/main/java/org/apache/commons/jexl3/scripting/JexlScriptEngine.java
+++ b/src/main/java/org/apache/commons/jexl3/scripting/JexlScriptEngine.java
@@ -121,8 +121,7 @@ public class JexlScriptEngine extends AbstractScriptEngine
implements Compilable
/**
* Wrapper to help convert a JSR-223 ScriptContext into a JexlContext.
- *
- * Current implementation only gives access to ENGINE_SCOPE binding.
+ * <p>The current implementation only gives access to ENGINE_SCOPE
binding.</p>
*/
private final class JexlContextWrapper implements JexlContext {
diff --git a/src/test/java/org/apache/commons/jexl3/ArithmeticTest.java
b/src/test/java/org/apache/commons/jexl3/ArithmeticTest.java
index 5a568fd4..ac4ad183 100644
--- a/src/test/java/org/apache/commons/jexl3/ArithmeticTest.java
+++ b/src/test/java/org/apache/commons/jexl3/ArithmeticTest.java
@@ -2238,7 +2238,8 @@ class ArithmeticTest extends JexlTestCase {
// JEXL-161
final JexlContext jc = new MapContext();
- jc.set("x", xml.getLastChild());
+ Object xmlx = xml.getLastChild();
+ jc.set("x", xmlx);
final String y = "456";
jc.set("y", y);
final JexlScript s = jexl.createScript("x.attribute.info = y");
@@ -2251,7 +2252,7 @@ class ArithmeticTest extends JexlTestCase {
assertEquals(y, info.getValue());
} catch (final JexlException.Property xprop) {
// test fails in java > 11 because modules, etc; need investigation
- assertTrue(xprop.getMessage().contains("info"));
+ assertTrue(xprop.getMessage().contains("attribute"));
assertTrue(getJavaVersion() > 11);
}
}
diff --git a/src/test/java/org/apache/commons/jexl3/Issues400Test.java
b/src/test/java/org/apache/commons/jexl3/Issues400Test.java
index 8b2a44ba..280fd6d6 100644
--- a/src/test/java/org/apache/commons/jexl3/Issues400Test.java
+++ b/src/test/java/org/apache/commons/jexl3/Issues400Test.java
@@ -1157,5 +1157,6 @@ public class Issues400Test {
template.evaluate(null, writer);
Assertions.assertEquals("42", writer.toString());
}
+
}
diff --git a/src/test/java/org/apache/commons/jexl3/SwitchTest.java
b/src/test/java/org/apache/commons/jexl3/SwitchTest.java
index 06483b61..7d2eb332 100644
--- a/src/test/java/org/apache/commons/jexl3/SwitchTest.java
+++ b/src/test/java/org/apache/commons/jexl3/SwitchTest.java
@@ -16,6 +16,7 @@
*/
package org.apache.commons.jexl3;
+import static
org.apache.commons.jexl3.introspection.JexlPermissions.RESTRICTED;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -136,6 +137,12 @@ public class SwitchTest extends JexlTestCase {
UNDEFINED, UNDECLARED, GLOBAL, LOCAL, THIS, SUPER;
}
+
+ @Test
+ void test440p() {
+ Assertions.assertTrue(RESTRICTED.allow(this.getClass()));
+ }
+
@Test
void test440c() {
final JexlEngine jexl = new
JexlBuilder().loader(getClass().getClassLoader()).imports(this.getClass().getName()).create();
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 37a8145a..9bf60b75 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
@@ -24,13 +24,13 @@ import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import java.io.FileWriter;
import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
-import java.util.HashSet;
import java.util.LinkedList;
import java.util.List;
import java.util.Map;
@@ -238,7 +238,7 @@ class PermissionsTest {
}
@Test
- void testParsePermissions0a() throws Exception {
+ void testParsePermissions0a() {
final String src = "java.lang { Runtime { exit(); exec(); }
}\njava.net { URL {} }";
final Permissions p = (Permissions) JexlPermissions.parse(src);
final Map<String, Permissions.NoJexlPackage> nojexlmap =
p.getPackages();
@@ -259,7 +259,7 @@ class PermissionsTest {
}
@Test
- void testParsePermissions0b() throws Exception {
+ void testParsePermissions0b() {
final String src = "java.lang { -Runtime { exit(); } }";
final Permissions p = (Permissions) JexlPermissions.parse(src);
final Method exit = getMethod(java.lang.Runtime.class,"exit");
@@ -268,7 +268,7 @@ class PermissionsTest {
}
@Test
- void testParsePermissions0c() throws Exception {
+ void testParsePermissions0c() {
final String src = "java.lang { +Runtime { availableProcessorCount();
} }";
final Permissions p = (Permissions) JexlPermissions.parse(src);
final Method exit = getMethod(java.lang.Runtime.class,"exit");
@@ -301,7 +301,7 @@ class PermissionsTest {
}
@Test
- void testParsePermissions0f() throws Exception {
+ void testParsePermissions0f() {
final String src = "java.lang { +Class { getName(); getSimpleName(); }
}";
final JexlPermissions p = RESTRICTED.compose(src);
final Method getName = getMethod(java.lang.Class.class,"getName");
@@ -323,7 +323,7 @@ class PermissionsTest {
}
@Test
- void testParsePermissions0g() throws Exception {
+ void testParsePermissions0g() {
final String src = "java.lang { +Class { } }";
final JexlPermissions p = RESTRICTED.compose(src);
final Method getName = getMethod(java.lang.Class.class,"getName");
@@ -341,23 +341,23 @@ class PermissionsTest {
@Test
void testParsePermissions1() {
final String[] src = {
- "java.lang.*",
- "java.math.*",
- "java.text.*",
- "java.util.*",
- "java.lang { Runtime {} }",
- "java.rmi {}",
- "java.io { File {} }",
- "java.nio { Path {} }" ,
- "org.apache.commons.jexl3.internal.introspection { " +
- "PermissionsTest { #level 0\n" +
- " Outer { #level 1\n" +
- " Inner { #level 2\n" +
- " callMeNot();" +
- " }" +
+ "java.lang.*",
+ "java.math.*",
+ "java.text.*",
+ "java.util.*",
+ "java.lang { Runtime {} }",
+ "java.rmi {}",
+ "java.io { File {} }",
+ "java.nio { Path {} }" ,
+ "org.apache.commons.jexl3.internal.introspection { " +
+ "PermissionsTest { #level 0\n" +
+ " Outer { #level 1\n" +
+ " Inner { #level 2\n" +
+ " callMeNot();" +
" }" +
" }" +
- " }"};
+ " }" +
+ " }"};
final Permissions p = (Permissions) JexlPermissions.parse(src);
final Map<String, Permissions.NoJexlPackage> nojexlmap =
p.getPackages();
assertNotNull(nojexlmap);
@@ -365,7 +365,8 @@ class PermissionsTest {
assertEquals(4, wildcards.size());
final JexlEngine jexl = new
JexlBuilder().permissions(p).safe(false).lexical(true).create();
-
+ final Class<?> longz = Long.TYPE;
+ assertTrue(p.allow(longz));
final Method exit = getMethod(java.lang.Runtime.class,"exit");
assertNotNull(exit);
assertFalse(p.allow(exit));
@@ -422,7 +423,7 @@ class PermissionsTest {
runTestPermissions(new
JexlPermissions.ClassPermissions(permissions0(), Collections.emptySet()));
}
- @Test void testPrivateOverload1() throws Exception {
+ @Test void testPrivateOverload1() {
final String src = "parseDouble(\"PHM1\".substring(3)).intValue()";
final JexlArithmetic jexla = new I33Arithmetic(true);
final JexlEngine jexl = new
JexlBuilder().safe(false).arithmetic(jexla).create();
@@ -491,7 +492,7 @@ class PermissionsTest {
void testWildCardPackages() {
Set<String> wildcards;
boolean found;
- wildcards = new HashSet<>(Arrays.asList("com.apache.*"));
+ wildcards = Collections.singleton("com.apache.*");
found = Permissions.wildcardAllow(wildcards,
"com.apache.commons.jexl3");
assertTrue(found);
found = Permissions.wildcardAllow(wildcards, "com.google.spexl");
@@ -534,4 +535,56 @@ class PermissionsTest {
final JexlScript script = jexl.createScript(src);
Assertions.assertThrows(JexlException.class, ()->
script.execute(null));
}
+
+ @Test
+ void testPermissions457a() {
+ Assertions.assertTrue(RESTRICTED.allow(Long.TYPE));
+ for (Constructor<?> ctor : FileWriter.class.getDeclaredConstructors())
{
+ Assertions.assertFalse(RESTRICTED.allow(ctor));
+ }
+ for (Method m : FileWriter.class.getMethods()) {
+ if (m.getName().equals("write")) {
+ Assertions.assertTrue(RESTRICTED.allow(m), m.toString());
+ }
+ }
+ }
+
+ @Test
+ void testPermissions457b() {
+ final JexlEngine jexl = new
JexlBuilder().silent(false).permissions(RESTRICTED).create();
+ List<String> srcs = Arrays.asList(
+ "new('java.io.FileWriter', 'test.txt')",
+ "new('java.io.FileWriter', 'test.txt', true)",
+ "new('java.io.FileWriter', new java.io.File('test.txt'))",
+ "new('java.io.FileWriter', new java.io.File('test.txt'), true)",
+ "import java.io.FileWriter; new FileWriter('test.txt')"
+ );
+ for (String src : srcs) {
+ final JexlScript script = jexl.createScript(src);
+ try {
+ script.execute(null);
+ Assertions.fail("should have thrown a permission exception");
+ } catch (JexlException.Method exception) {
+ Assertions.assertTrue( exception.getMethod().contains("File"),
"FileWriter::new should not be allowed");
+ }
+ }
+ }
+
+ @Test
+ void testPermissions457c() {
+ final JexlEngine jexl = new
JexlBuilder().silent(false).permissions(RESTRICTED).create();
+ List<String> srcs = Arrays.asList(
+ "new('java.io.FileReader','test.txt')",
+ "new('java.io.FileReader', new java.io.File('test.txt'))",
+ "import java.io.FileReader; new FileReader('test.txt')");
+ for (String src : srcs) {
+ final JexlScript script = jexl.createScript(src);
+ try {
+ script.execute(null);
+ Assertions.fail("should have thrown a permission exception");
+ } catch (JexlException.Method exception) {
+ Assertions.assertTrue( exception.getMethod().contains("File"),
"FileReader::new should not be allowed");
+ }
+ }
+ }
}
