You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by he...@apache.org on 2023/02/07 15:13:16 UTC
[commons-jexl] branch master updated: JEXL-381: added missing permissions checks; - allow permission compositions;
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 21dd3ca0 JEXL-381: added missing permissions checks; - allow permission compositions;
21dd3ca0 is described below
commit 21dd3ca01b0cbfae500cab527abe471a39995914
Author: Henri Biestro <hb...@cloudera.com>
AuthorDate: Tue Feb 7 15:58:48 2023 +0100
JEXL-381: added missing permissions checks;
- allow permission compositions;
---
pom.xml | 6 ++
.../internal/introspection/DuckGetExecutor.java | 2 +-
.../internal/introspection/DuckSetExecutor.java | 2 +-
.../jexl3/internal/introspection/Introspector.java | 2 +-
.../internal/introspection/ListGetExecutor.java | 3 +-
.../internal/introspection/ListSetExecutor.java | 4 +-
.../internal/introspection/MapGetExecutor.java | 3 +-
.../internal/introspection/MapSetExecutor.java | 3 +-
.../jexl3/internal/introspection/Permissions.java | 22 +++++-
.../internal/introspection/PermissionsParser.java | 15 ++++-
.../jexl3/internal/introspection/Uberspect.java | 10 +--
.../jexl3/introspection/JexlPermissions.java | 12 +++-
.../commons/jexl3/ComposePermissionsTest.java | 78 ++++++++++++++++++++++
.../org/apache/commons/jexl3/JexlTestCase.java | 30 ++-------
src/test/scripts/sample.json | 18 +++++
15 files changed, 169 insertions(+), 41 deletions(-)
diff --git a/pom.xml b/pom.xml
index b655a9ee..4c413ecd 100644
--- a/pom.xml
+++ b/pom.xml
@@ -113,6 +113,12 @@
<artifactId>junit-vintage-engine</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>com.google.code.gson</groupId>
+ <artifactId>gson</artifactId>
+ <version>2.10.1</version>
+ <scope>test</scope>
+ </dependency>
</dependencies>
<build>
diff --git a/src/main/java/org/apache/commons/jexl3/internal/introspection/DuckGetExecutor.java b/src/main/java/org/apache/commons/jexl3/internal/introspection/DuckGetExecutor.java
index ca267d50..409bb1b6 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/introspection/DuckGetExecutor.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/introspection/DuckGetExecutor.java
@@ -42,7 +42,7 @@ public final class DuckGetExecutor extends AbstractExecutor.Get {
* @return the executor if found, null otherwise
*/
public static DuckGetExecutor discover(final Introspector is, final Class<?> clazz, final Object identifier) {
- final java.lang.reflect.Method method = is.getMethod(clazz, "get", makeArgs(identifier));
+ final java.lang.reflect.Method method = is.getMethod(clazz, "get", identifier);
return method == null? null : new DuckGetExecutor(clazz, method, identifier);
}
diff --git a/src/main/java/org/apache/commons/jexl3/internal/introspection/DuckSetExecutor.java b/src/main/java/org/apache/commons/jexl3/internal/introspection/DuckSetExecutor.java
index a7b35f44..d8e5d9d2 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/introspection/DuckSetExecutor.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/introspection/DuckSetExecutor.java
@@ -52,7 +52,7 @@ public final class DuckSetExecutor extends AbstractExecutor.Set {
* @return the executor if found, null otherwise
*/
public static DuckSetExecutor discover(final Introspector is, final Class<?> clazz, final Object key, final Object value) {
- java.lang.reflect.Method method = is.getMethod(clazz, "set", makeArgs(key, value));
+ java.lang.reflect.Method method = is.getMethod(clazz, "set", key, value);
if (method == null) {
method = is.getMethod(clazz, "put", makeArgs(key, value));
}
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 82945c47..1e2e417a 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
@@ -131,7 +131,7 @@ public final class Introspector {
* @return the desired method object
* @throws MethodKey.AmbiguousException if no unambiguous method could be found through introspection
*/
- public Method getMethod(final Class<?> c, final String name, final Object[] params) {
+ public Method getMethod(final Class<?> c, final String name, final Object... params) {
return getMethod(c, new MethodKey(name, params));
}
diff --git a/src/main/java/org/apache/commons/jexl3/internal/introspection/ListGetExecutor.java b/src/main/java/org/apache/commons/jexl3/internal/introspection/ListGetExecutor.java
index 7e02b5f5..866b1e67 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/introspection/ListGetExecutor.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/introspection/ListGetExecutor.java
@@ -46,7 +46,8 @@ public final class ListGetExecutor extends AbstractExecutor.Get {
if (clazz.isArray()) {
return new ListGetExecutor(clazz, ARRAY_GET, index);
}
- if (List.class.isAssignableFrom(clazz)) {
+ // we still need to ensure permissions grant access to get(...)
+ if (List.class.isAssignableFrom(clazz) && is.getMethod(clazz, "get", index) != null) {
return new ListGetExecutor(clazz, LIST_GET, index);
}
}
diff --git a/src/main/java/org/apache/commons/jexl3/internal/introspection/ListSetExecutor.java b/src/main/java/org/apache/commons/jexl3/internal/introspection/ListSetExecutor.java
index 4c6444b4..105b467a 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/introspection/ListSetExecutor.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/introspection/ListSetExecutor.java
@@ -57,7 +57,9 @@ public final class ListSetExecutor extends AbstractExecutor.Set {
return new ListSetExecutor(clazz, ARRAY_SET, index);
// }
}
- if (List.class.isAssignableFrom(clazz)) {
+ // we still need to ensure permissions grant access to set(...)
+ if (List.class.isAssignableFrom(clazz)
+ && is.getMethod(clazz, "set", index, value) != null) {
return new ListSetExecutor(clazz, LIST_SET, index);
}
}
diff --git a/src/main/java/org/apache/commons/jexl3/internal/introspection/MapGetExecutor.java b/src/main/java/org/apache/commons/jexl3/internal/introspection/MapGetExecutor.java
index adf80291..c620100f 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/introspection/MapGetExecutor.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/introspection/MapGetExecutor.java
@@ -39,7 +39,8 @@ public final class MapGetExecutor extends AbstractExecutor.Get {
* @return the executor if found, null otherwise
*/
public static MapGetExecutor discover(final Introspector is, final Class<?> clazz, final Object identifier) {
- if (Map.class.isAssignableFrom(clazz)) {
+ // we still need to ensure permissions grant access to get(...)
+ if (Map.class.isAssignableFrom(clazz) && is.getMethod(clazz, "get", identifier) != null) {
return new MapGetExecutor(clazz, MAP_GET, identifier);
}
return null;
diff --git a/src/main/java/org/apache/commons/jexl3/internal/introspection/MapSetExecutor.java b/src/main/java/org/apache/commons/jexl3/internal/introspection/MapSetExecutor.java
index e3d00811..27b80bc7 100644
--- a/src/main/java/org/apache/commons/jexl3/internal/introspection/MapSetExecutor.java
+++ b/src/main/java/org/apache/commons/jexl3/internal/introspection/MapSetExecutor.java
@@ -43,7 +43,8 @@ public final class MapSetExecutor extends AbstractExecutor.Set {
final Class<?> clazz,
final Object identifier,
final Object value) {
- if (Map.class.isAssignableFrom(clazz)) {
+ // we still need to ensure permissions grant access to put(...)
+ if (Map.class.isAssignableFrom(clazz) && is.getMethod(clazz, "put", identifier, value) != null) {
return new MapSetExecutor(clazz, MAP_SET, identifier, value);
}
return null;
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 ff7e3eea..d6a134dd 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
@@ -23,6 +23,7 @@ import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.util.Collections;
import java.util.HashSet;
+import java.util.LinkedHashSet;
import java.util.Map;
import java.util.Objects;
import java.util.Set;
@@ -223,6 +224,16 @@ public class Permissions implements JexlPermissions {
this.packages = nojexl;
}
+ /**
+ * Creates a new set of permissions by composing these permissions with a new set of rules.
+ * @param src the rules
+ * @return the new permissions
+ */
+ @Override
+ public Permissions compose(final String... src) {
+ return new PermissionsParser().parse(new LinkedHashSet<>(allowed),new ConcurrentHashMap<>(packages), src);
+ }
+
/**
* The no-restriction introspection permission singleton.
*/
@@ -399,15 +410,20 @@ public class Permissions implements JexlPermissions {
if (deny(clazz)) {
return false;
}
- // all super classes must be allowed
+ // no super class can be denied and at least one must be allowed
+ boolean explicit = wildcardAllow(clazz);
Class<?> walk = clazz.getSuperclass();
while (walk != null) {
if (deny(walk)) {
return false;
}
+ if (!explicit) {
+ explicit = wildcardAllow(walk);
+ }
walk = walk.getSuperclass();
}
- return true;
+ // check wildcards
+ return explicit;
}
/**
@@ -486,7 +502,7 @@ public class Permissions implements JexlPermissions {
}
Class<?> clazz = method.getDeclaringClass();
// gather if any implementation of the method is explicitly allowed by the packages
- final boolean[] explicit = {wildcardAllow(clazz)};
+ final boolean[] explicit = { wildcardAllow(clazz) };
// lets walk all interfaces
for (final Class<?> inter : clazz.getInterfaces()) {
if (!allow(inter, method, explicit)) {
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 0ed25da8..ad6ec0c1 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
@@ -75,11 +75,22 @@ public class PermissionsParser {
* @return the permissions map
*/
public Permissions parse(final String... srcs) {
+ return parse(new LinkedHashSet<>(), new ConcurrentHashMap<>(), srcs);
+ }
+
+ /**
+ * Parses permissions from a source.
+ * @param wildcards the set of allowed packages
+ * @param packages the map of restricted elements
+ * @param srcs the sources
+ * @return the permissions map
+ */
+ Permissions parse(Set<String> wildcards, Map<String, Permissions.NoJexlPackage> packages, final String... srcs) {
if (srcs == null || srcs.length == 0) {
return Permissions.UNRESTRICTED;
}
- packages = new ConcurrentHashMap<>();
- wildcards = new LinkedHashSet<>();
+ this.packages = packages;
+ this.wildcards = wildcards;
for(final String src : srcs) {
this.src = src;
this.size = src.length();
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 a7ca53d0..cb077b04 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
@@ -109,7 +109,7 @@ public class Uberspect implements JexlUberspect {
protected final Introspector base() {
Introspector intro = ref.get();
if (intro == null) {
- // double checked locking is ok (fixed by Java 5 memory model).
+ // double-checked locking is ok (fixed by Java 5 memory model).
synchronized (this) {
intro = ref.get();
if (intro == null) {
@@ -190,8 +190,7 @@ public class Uberspect implements JexlUberspect {
*
* @param c Class in which the method search is taking place
* @param name Name of the method being searched for
- * @param params An array of Objects (not Classes) that describe the
- * the parameters
+ * @param params An array of Objects (not Classes) that describe the parameters
*
* @return a {@link java.lang.reflect.Method}
* or null if no unambiguous method could be found through introspection.
@@ -338,7 +337,7 @@ public class Uberspect implements JexlUberspect {
executor = MapSetExecutor.discover(is, claz, identifier, arg);
break;
case LIST:
- // let's see if we can convert the identifier to an int,
+ // let's see if we can convert the identifier to an int,
// if obj is an array or a list, we can still do something
final Integer index = AbstractExecutor.castInteger(identifier);
if (index != null) {
@@ -373,6 +372,9 @@ public class Uberspect implements JexlUberspect {
@Override
@SuppressWarnings("unchecked")
public Iterator<?> getIterator(final Object obj) {
+ if (!permissions.allow(obj.getClass())) {
+ return null;
+ }
if (obj instanceof Iterator<?>) {
return ((Iterator<?>) obj);
}
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 0ab4d64f..565f07d3 100644
--- a/src/main/java/org/apache/commons/jexl3/introspection/JexlPermissions.java
+++ b/src/main/java/org/apache/commons/jexl3/introspection/JexlPermissions.java
@@ -16,6 +16,7 @@
*/
package org.apache.commons.jexl3.introspection;
+import org.apache.commons.jexl3.internal.introspection.Permissions;
import org.apache.commons.jexl3.internal.introspection.PermissionsParser;
import java.lang.reflect.Constructor;
@@ -168,12 +169,21 @@ public interface JexlPermissions {
return new PermissionsParser().parse(src);
}
+ /**
+ * Compose these permissions with a new set.
+ * <p>This is a convenience method meant to easily give access to the packages JEXL is
+ * used to integrate with.</p>
+ * @param src the new constraints
+ * @return the new permissions
+ */
+ JexlPermissions compose(final String... 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
*/
- JexlPermissions UNRESTRICTED = JexlPermissions.parse(null);
+ JexlPermissions UNRESTRICTED = Permissions.UNRESTRICTED;
/**
* A restricted singleton.
* <p>The RESTRICTED set is built using the following allowed packages and denied packages/classes.</p>
diff --git a/src/test/java/org/apache/commons/jexl3/ComposePermissionsTest.java b/src/test/java/org/apache/commons/jexl3/ComposePermissionsTest.java
new file mode 100644
index 00000000..5f1a66b6
--- /dev/null
+++ b/src/test/java/org/apache/commons/jexl3/ComposePermissionsTest.java
@@ -0,0 +1,78 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.commons.jexl3;
+
+import java.io.File;
+import java.io.FileReader;
+
+import com.google.gson.Gson;
+import org.apache.commons.jexl3.introspection.JexlPermissions;
+import org.junit.Assert;
+import org.junit.Test;
+
+/**
+ * Tests for pragmas
+ */
+public class ComposePermissionsTest extends JexlTestCase {
+ static final String SAMPLE_JSON = "src/test/scripts/sample.json";
+ /**
+ * Create a new test case.
+ */
+ public ComposePermissionsTest() {
+ super("PermissionsTest");
+ }
+
+ @Test public void testComposePermissions() throws Exception {
+ final String check = "http://example.com/content.jpg";
+ final File jsonFile = new File(SAMPLE_JSON);
+ Gson gson = new Gson();
+ Object json = gson.fromJson(new FileReader(jsonFile), Object.class);
+ Assert.assertNotNull(json);
+
+ // will succeed because java.util.Map is allowed and gson LinkedTreeMap is one
+ JexlEngine j0 = createEngine(false, JexlPermissions.UNRESTRICTED);
+ JexlScript s0 = j0.createScript("json.pageInfo.pagePic", "json");
+ Object r0 = s0.execute(null, json);
+ Assert.assertEquals(check, r0);
+
+ // will fail if gson package is denied
+ JexlEngine j1 = createEngine(false, JexlPermissions.UNRESTRICTED.compose("com.google.gson.internal {}"));
+ JexlScript s1 = j1.createScript("json.pageInfo.pagePic", "json");
+ try {
+ Object r1 = s1.execute(null, json);
+ Assert.fail("gson restricted");
+ } catch (JexlException.Property xproperty) {
+ Assert.assertEquals("pageInfo", xproperty.getProperty());
+ }
+
+ // will fail since gson package is denied
+ j1 = createEngine(false, JexlPermissions.UNRESTRICTED.compose("com.google.gson.internal { LinkedTreeMap {} }"));
+ s1 = j1.createScript("json.pageInfo.pagePic", "json");
+ try {
+ Object r1 = s1.execute(null, json);
+ Assert.fail("gson LinkTreeMap restricted");
+ } catch (JexlException.Property xproperty) {
+ Assert.assertEquals("pageInfo", xproperty.getProperty());
+ }
+
+ // will not fail since gson package is not explicitely allowed
+ j1 = createEngine(false, JexlPermissions.RESTRICTED);
+ s1 = j1.createScript("json.pageInfo.pagePic", "json");
+ Object r1 = s1.execute(null, json);
+ Assert.assertEquals(check, r0);
+ }
+}
diff --git a/src/test/java/org/apache/commons/jexl3/JexlTestCase.java b/src/test/java/org/apache/commons/jexl3/JexlTestCase.java
index 62a45d7d..4ab80535 100644
--- a/src/test/java/org/apache/commons/jexl3/JexlTestCase.java
+++ b/src/test/java/org/apache/commons/jexl3/JexlTestCase.java
@@ -136,33 +136,15 @@ public class JexlTestCase {
/**
* A very secure singleton.
*/
- public static final JexlPermissions SECURE = JexlPermissions.parse(
- "# Secure 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 { } }"
- );
+ public static final JexlPermissions SECURE = JexlPermissions.RESTRICTED;
public static JexlEngine createEngine(final boolean lenient) {
+ return createEngine(lenient, SECURE);
+ }
+
+ public static JexlEngine createEngine(final boolean lenient, JexlPermissions permissions) {
return new JexlBuilder()
- .uberspect(new Uberspect(null, null, SECURE))
+ .uberspect(new Uberspect(null, null, permissions))
.arithmetic(new JexlArithmetic(!lenient)).cache(128).create();
}
diff --git a/src/test/scripts/sample.json b/src/test/scripts/sample.json
new file mode 100644
index 00000000..3dba3077
--- /dev/null
+++ b/src/test/scripts/sample.json
@@ -0,0 +1,18 @@
+{
+ "pageInfo": {
+ "pageName": "abc",
+ "pagePic": "http://example.com/content.jpg"
+ },
+ "posts": [
+ {
+ "post_id": "123456789012_123456789012",
+ "actor_id": "1234567890",
+ "picOfPersonWhoPosted": "http://example.com/photo.jpg",
+ "nameOfPersonWhoPosted": "Jane Doe",
+ "message": "Sounds cool. Can't wait to see it!",
+ "likesCount": "2",
+ "comments": [],
+ "timeOfPost": "1234567890"
+ }
+ ]
+}
\ No newline at end of file