You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@unomi.apache.org by jk...@apache.org on 2020/11/16 13:13:07 UTC
[unomi] branch unomi-1.5.x updated: UNOMI-399: load allow/forbid
script from files instead of configuration property to avoid script code
conflicting with property value parsing (#213)
This is an automated email from the ASF dual-hosted git repository.
jkevan pushed a commit to branch unomi-1.5.x
in repository https://gitbox.apache.org/repos/asf/unomi.git
The following commit(s) were added to refs/heads/unomi-1.5.x by this push:
new f78b9d2 UNOMI-399: load allow/forbid script from files instead of configuration property to avoid script code conflicting with property value parsing (#213)
f78b9d2 is described below
commit f78b9d2ac71246c6c1e5adabd75326003bad21b7
Author: kevan Jahanshahi <ke...@jahia.com>
AuthorDate: Mon Nov 16 13:17:34 2020 +0100
UNOMI-399: load allow/forbid script from files instead of configuration property to avoid script code conflicting with property value parsing (#213)
---
.../main/resources/etc/custom.system.properties | 8 ++--
package/src/main/resources/etc/mvel-allow.json | 1 +
package/src/main/resources/etc/mvel-forbid.json | 19 ++++++++
package/src/main/resources/etc/ognl-allow.json | 1 +
package/src/main/resources/etc/ognl-forbid.json | 19 ++++++++
scripting/pom.xml | 5 +++
.../internal/ExpressionFilterFactoryImpl.java | 51 +++++++++-------------
7 files changed, 69 insertions(+), 35 deletions(-)
diff --git a/package/src/main/resources/etc/custom.system.properties b/package/src/main/resources/etc/custom.system.properties
index 9660b68..2c9b395 100644
--- a/package/src/main/resources/etc/custom.system.properties
+++ b/package/src/main/resources/etc/custom.system.properties
@@ -48,10 +48,10 @@ org.apache.unomi.scripting.filter.activated=${env:UNOMI_SCRIPTING_FILTER_ACTIVAT
# It is however fully expected to add new expressions to the "allow" value, although it is better to add them inside
# any plugins you may be adding. This configuration is only designed to compensate for the cases where something was not properly designed or to deal with compatibility issues. Just be VERY careful to make your patterns AS SPECIFIC AS POSSIBLE in order to avoid introducing a way to abuse the expression filtering.
org.apache.unomi.scripting.filter.collections=${env:UNOMI_SCRIPTING_FILTER_COLLECTIONS:-mvel,ognl}
-org.apache.unomi.scripting.filter.mvel.allow=${env:UNOMI_SCRIPTING_FILTER_MVEL_ALLOW:-}
-org.apache.unomi.scripting.filter.mvel.forbid=${env:UNOMI_SCRIPTING_FILTER_MVEL_FORBID:-.*Runtime.*,.*ProcessBuilder.*,.*exec.*,.*invoke.*,.*getClass.*,.*Class.*,.*ClassLoader.*,.*System.*,.*Method.*,.*method.*,.*Compiler.*,.*Thread.*,.*FileWriter.*,.*forName.*,.*Socket.*,.*DriverManager.*,eval}
-org.apache.unomi.scripting.filter.ognl.allow=${env:UNOMI_SCRIPTING_FILTER_OGNL_ALLOW:-}
-org.apache.unomi.scripting.filter.ognl.forbid=${env:UNOMI_SCRIPTING_FILTER_OGNL_FORBID:-.*Runtime.*,.*ProcessBuilder.*,.*exec.*,.*invoke.*,.*getClass.*,.*Class.*,.*ClassLoader.*,.*System.*,.*Method.*,.*method.*,.*Compiler.*,.*Thread.*,.*FileWriter.*,.*forName.*,.*Socket.*,.*DriverManager.*,eval}
+org.apache.unomi.scripting.filter.mvel.allow=${env:UNOMI_SCRIPTING_FILTER_MVEL_ALLOW:-${karaf.etc}/mvel-allow.json}
+org.apache.unomi.scripting.filter.mvel.forbid=${env:UNOMI_SCRIPTING_FILTER_MVEL_FORBID:-${karaf.etc}/mvel-forbid.json}
+org.apache.unomi.scripting.filter.ognl.allow=${env:UNOMI_SCRIPTING_FILTER_OGNL_ALLOW:-${karaf.etc}/ognl-allow.json}
+org.apache.unomi.scripting.filter.ognl.forbid=${env:UNOMI_SCRIPTING_FILTER_OGNL_FORBID:-${karaf.etc}/ognl-forbid.json}
# This parameter controls whether OGNL scripting is allowed in expressions. Because of security reasons it is
# deactivated by default. If you run into compatibility issues you could reactivate it but it is at your own risk.
diff --git a/package/src/main/resources/etc/mvel-allow.json b/package/src/main/resources/etc/mvel-allow.json
new file mode 100644
index 0000000..0637a08
--- /dev/null
+++ b/package/src/main/resources/etc/mvel-allow.json
@@ -0,0 +1 @@
+[]
\ No newline at end of file
diff --git a/package/src/main/resources/etc/mvel-forbid.json b/package/src/main/resources/etc/mvel-forbid.json
new file mode 100644
index 0000000..07e8ee6
--- /dev/null
+++ b/package/src/main/resources/etc/mvel-forbid.json
@@ -0,0 +1,19 @@
+[
+ ".*Runtime.*",
+ ".*ProcessBuilder.*",
+ ".*exec.*",
+ ".*invoke.*",
+ ".*getClass.*",
+ ".*Class.*",
+ ".*ClassLoader.*",
+ ".*System.*",
+ ".*Method.*",
+ ".*method.*",
+ ".*Compiler.*",
+ ".*Thread.*",
+ ".*FileWriter.*",
+ ".*forName.*",
+ ".*Socket.*",
+ ".*DriverManager.*",
+ "eval"
+]
\ No newline at end of file
diff --git a/package/src/main/resources/etc/ognl-allow.json b/package/src/main/resources/etc/ognl-allow.json
new file mode 100644
index 0000000..0637a08
--- /dev/null
+++ b/package/src/main/resources/etc/ognl-allow.json
@@ -0,0 +1 @@
+[]
\ No newline at end of file
diff --git a/package/src/main/resources/etc/ognl-forbid.json b/package/src/main/resources/etc/ognl-forbid.json
new file mode 100644
index 0000000..07e8ee6
--- /dev/null
+++ b/package/src/main/resources/etc/ognl-forbid.json
@@ -0,0 +1,19 @@
+[
+ ".*Runtime.*",
+ ".*ProcessBuilder.*",
+ ".*exec.*",
+ ".*invoke.*",
+ ".*getClass.*",
+ ".*Class.*",
+ ".*ClassLoader.*",
+ ".*System.*",
+ ".*Method.*",
+ ".*method.*",
+ ".*Compiler.*",
+ ".*Thread.*",
+ ".*FileWriter.*",
+ ".*forName.*",
+ ".*Socket.*",
+ ".*DriverManager.*",
+ "eval"
+]
\ No newline at end of file
diff --git a/scripting/pom.xml b/scripting/pom.xml
index 2a93b19..72d9e3b 100644
--- a/scripting/pom.xml
+++ b/scripting/pom.xml
@@ -43,6 +43,11 @@
</dependency>
<dependency>
+ <groupId>org.apache.commons</groupId>
+ <artifactId>commons-lang3</artifactId>
+ </dependency>
+
+ <dependency>
<groupId>org.apache.unomi</groupId>
<artifactId>unomi-api</artifactId>
<version>1.5.4-SNAPSHOT</version>
diff --git a/scripting/src/main/java/org/apache/unomi/scripting/internal/ExpressionFilterFactoryImpl.java b/scripting/src/main/java/org/apache/unomi/scripting/internal/ExpressionFilterFactoryImpl.java
index 6278fe3..5831842 100644
--- a/scripting/src/main/java/org/apache/unomi/scripting/internal/ExpressionFilterFactoryImpl.java
+++ b/scripting/src/main/java/org/apache/unomi/scripting/internal/ExpressionFilterFactoryImpl.java
@@ -18,6 +18,7 @@ package org.apache.unomi.scripting.internal;
import com.fasterxml.jackson.databind.JsonNode;
import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.commons.lang3.StringUtils;
import org.apache.unomi.scripting.ExpressionFilter;
import org.apache.unomi.scripting.ExpressionFilterFactory;
import org.osgi.framework.Bundle;
@@ -27,6 +28,7 @@ import org.osgi.framework.BundleListener;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import java.io.File;
import java.io.IOException;
import java.net.URL;
import java.util.*;
@@ -58,37 +60,8 @@ public class ExpressionFilterFactoryImpl implements ExpressionFilterFactory,Bund
String[] initialFilterCollectionParts = initialFilterCollections.split(",");
if (initialFilterCollectionParts != null) {
for (String initialFilterCollection : initialFilterCollectionParts) {
- String systemAllowedPatterns = System.getProperty("org.apache.unomi.scripting.filter."+initialFilterCollection+".allow", null);
- if (systemAllowedPatterns != null) {
- Set<Pattern> collectionAllowedExpressionPatterns = new HashSet<>();
- if ("all".equals(systemAllowedPatterns.trim())) {
- collectionAllowedExpressionPatterns = null;
- } else {
- if (systemAllowedPatterns.trim().length() > 0) {
- String[] systemAllowedPatternParts = systemAllowedPatterns.split(",");
- collectionAllowedExpressionPatterns = new HashSet<>();
- for (String systemAllowedPatternPart : systemAllowedPatternParts) {
- collectionAllowedExpressionPatterns.add(Pattern.compile(systemAllowedPatternPart));
- }
- }
- }
- allowedExpressionPatternsByCollection.put(initialFilterCollection, collectionAllowedExpressionPatterns);
- }
-
- String systemForbiddenPatterns = System.getProperty("org.apache.unomi.scripting.filter."+initialFilterCollection+".forbid", ".*Runtime.*,.*ProcessBuilder.*,.*exec.*,.*invoke.*,.*getClass.*,.*Class.*,.*ClassLoader.*,.*System.*,.*Method.*,.*method.*,.*Compiler.*,.*Thread.*,.*FileWriter.*,.*forName.*,.*Socket.*,.*DriverManager.*,eval");
- if (systemForbiddenPatterns != null) {
- Set<Pattern> collectionForbiddenExpressionPatterns = new HashSet<>();
- if (systemForbiddenPatterns.trim().length() > 0) {
- String[] systemForbiddenPatternParts = systemForbiddenPatterns.split(",");
- collectionForbiddenExpressionPatterns = new HashSet<>();
- for (String systemForbiddenPatternPart : systemForbiddenPatternParts) {
- collectionForbiddenExpressionPatterns.add(Pattern.compile(systemForbiddenPatternPart));
- }
- } else {
- collectionForbiddenExpressionPatterns = null;
- }
- forbiddenExpressionPatternsByCollection.put(initialFilterCollection, collectionForbiddenExpressionPatterns);
- }
+ allowedExpressionPatternsByCollection.put(initialFilterCollection, loadPatternsFromConfig("org.apache.unomi.scripting.filter."+initialFilterCollection+".allow"));
+ forbiddenExpressionPatternsByCollection.put(initialFilterCollection, loadPatternsFromConfig("org.apache.unomi.scripting.filter."+initialFilterCollection+".forbid"));
}
}
@@ -102,7 +75,23 @@ public class ExpressionFilterFactoryImpl implements ExpressionFilterFactory,Bund
bundleContext.addBundleListener(this);
}
+ }
+ private Set<Pattern> loadPatternsFromConfig(String propertyKey) {
+ String patternsFile = System.getProperty(propertyKey, null);
+ if (StringUtils.isNotEmpty(patternsFile)) {
+ Set<Pattern> patterns = new HashSet<>();
+ try {
+ JsonNode jsonPatterns = objectMapper.readTree(new File(patternsFile));
+ for (JsonNode jsonPattern : jsonPatterns) {
+ patterns.add(Pattern.compile(jsonPattern.asText()));
+ }
+ } catch (IOException e) {
+ logger.error("Error while loading expressions definition from " + propertyKey, e);
+ }
+ return patterns;
+ }
+ return null;
}
public void destroy() {