You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@unomi.apache.org by sh...@apache.org on 2020/08/24 08:09:58 UTC
[unomi] branch unomi-1.5.x updated: Improve scripting security
(#179)
This is an automated email from the ASF dual-hosted git repository.
shuber 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 0b81ba3 Improve scripting security (#179)
0b81ba3 is described below
commit 0b81ba35dd3c3c2e0a92ce06592b3df90571eced
Author: Serge Huber <sh...@jahia.com>
AuthorDate: Thu Aug 20 15:07:40 2020 +0200
Improve scripting security (#179)
* Improve scripting security:
- OGNL is now disabled by default as it wasn't really used much (possible to reactive through a system setting)
- A new condition sanitizer has been added in the ContextServlet to filter out any MVEL scripts (again not used much and can be reactivate using a system property)
- A new ExpressionFilter has been added that will use configurable (system property) regular expressions to filter out possible malicious expressions
- OGNL sandboxing has been improved
- MVEL default imports have been modified to prevent using system-level classes
* Fix bug in sanitizing code
* New scripting execution sub-system:
- Allow-listing of allowed expressions
- Plugins may deployed their own allow-lists using JSON files
- OGNL scripting is now deactivated by default
- Minimal list of built-in MVEL allowed patterns
* Fix typo in marker
(cherry picked from commit 343d24f27b1790701225711491e967bd3e2111c8)
---
common/pom.xml | 36 ++++
.../test/java/org/apache/unomi/itests/BaseIT.java | 37 +++-
.../org/apache/unomi/itests/ContextServletIT.java | 57 ++++++-
.../test/resources/security/mvel-payload-1.json | 20 +++
.../test/resources/security/ognl-payload-1.json | 28 +++
kar/pom.xml | 5 +
kar/src/main/feature/feature.xml | 1 +
.../main/resources/etc/custom.system.properties | 7 +-
persistence-elasticsearch/core/pom.xml | 11 +-
.../conditions/ConditionContextHelper.java | 42 ++---
.../ConditionESQueryBuilderDispatcher.java | 10 +-
.../conditions/ConditionEvaluatorDispatcher.java | 8 +-
.../resources/OSGI-INF/blueprint/blueprint.xml | 3 +
plugins/baseplugin/pom.xml | 6 +
.../PastEventConditionESQueryBuilder.java | 8 +-
.../conditions/PastEventConditionEvaluator.java | 9 +-
.../conditions/PropertyConditionEvaluator.java | 116 ++++++++++++-
.../resources/META-INF/cxs/expressions/mvel.json | 13 ++
.../resources/OSGI-INF/blueprint/blueprint.xml | 3 +
.../conditions/PropertyConditionEvaluatorTest.java | 34 +++-
pom.xml | 3 +-
{plugins/baseplugin => scripting}/pom.xml | 89 ++++------
.../apache/unomi/scripting/ExpressionFilter.java | 59 +++++++
.../unomi/scripting/ExpressionFilterFactory.java | 31 ++++
.../apache/unomi/scripting/MvelScriptExecutor.java | 80 +++++++++
.../org/apache/unomi/scripting/ScriptExecutor.java | 28 +++
.../scripting}/SecureFilteringClassLoader.java | 22 ++-
.../internal/ExpressionFilterFactoryImpl.java | 190 +++++++++++++++++++++
.../resources/META-INF/cxs/expressions/mvel.json | 1 +
.../resources/META-INF/cxs/expressions/ognl.json | 1 +
.../resources/OSGI-INF/blueprint/blueprint.xml | 48 ++++++
.../unomi/scripting/MvelScriptExecutorTest.java | 130 ++++++++++++++
services/pom.xml | 12 +-
.../services/actions/ActionExecutorDispatcher.java | 36 ++--
.../resources/OSGI-INF/blueprint/blueprint.xml | 4 +-
.../actions/ActionExecutorDispatcherTest.java | 106 ------------
.../resources/OSGI-INF/blueprint/blueprint.xml | 1 +
.../java/org/apache/unomi/web/ContextServlet.java | 93 +++++++++-
38 files changed, 1120 insertions(+), 268 deletions(-)
diff --git a/common/pom.xml b/common/pom.xml
index 36cfb5f..55e3d7d 100644
--- a/common/pom.xml
+++ b/common/pom.xml
@@ -54,6 +54,42 @@
<version>${version.karaf}</version>
<scope>provided</scope>
</dependency>
+
+ <dependency>
+ <groupId>org.apache.unomi</groupId>
+ <artifactId>unomi-api</artifactId>
+ <version>2.0.0-SNAPSHOT</version>
+ <scope>test</scope>
+ </dependency>
+
+ <dependency>
+ <groupId>org.slf4j</groupId>
+ <artifactId>slf4j-simple</artifactId>
+ <version>1.6.6</version>
+ <scope>test</scope>
+ </dependency>
+
</dependencies>
+ <build>
+ <plugins>
+ <plugin>
+ <groupId>org.apache.felix</groupId>
+ <artifactId>maven-bundle-plugin</artifactId>
+ <extensions>true</extensions>
+ <configuration>
+ <instructions>
+ <Embed-Dependency>*;scope=compile|runtime</Embed-Dependency>
+ <Import-Package>
+ org.apache.unomi.api,
+ org.apache.unomi.api.conditions,
+ org.apache.unomi.api.goals,
+ *
+ </Import-Package>
+ </instructions>
+ </configuration>
+ </plugin>
+ </plugins>
+ </build>
+
</project>
diff --git a/itests/src/test/java/org/apache/unomi/itests/BaseIT.java b/itests/src/test/java/org/apache/unomi/itests/BaseIT.java
index f6c35e4..bc2df79 100644
--- a/itests/src/test/java/org/apache/unomi/itests/BaseIT.java
+++ b/itests/src/test/java/org/apache/unomi/itests/BaseIT.java
@@ -17,9 +17,12 @@
package org.apache.unomi.itests;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.commons.io.IOUtils;
import org.apache.unomi.api.Item;
import org.apache.unomi.api.conditions.Condition;
import org.apache.unomi.api.services.DefinitionsService;
+import org.apache.unomi.persistence.spi.CustomObjectMapper;
import org.apache.unomi.persistence.spi.PersistenceService;
import org.junit.Assert;
import org.ops4j.pax.exam.Configuration;
@@ -30,23 +33,21 @@ import org.ops4j.pax.exam.karaf.options.LogLevelOption.LogLevel;
import org.ops4j.pax.exam.options.MavenArtifactUrlReference;
import org.ops4j.pax.exam.options.extra.VMOption;
import org.ops4j.pax.exam.util.Filter;
+import org.osgi.framework.BundleContext;
import javax.inject.Inject;
import java.io.File;
+import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
+import java.util.Map;
import java.util.function.Predicate;
import java.util.function.Supplier;
import static org.ops4j.pax.exam.CoreOptions.maven;
import static org.ops4j.pax.exam.CoreOptions.systemProperty;
-import static org.ops4j.pax.exam.karaf.options.KarafDistributionOption.debugConfiguration;
-import static org.ops4j.pax.exam.karaf.options.KarafDistributionOption.editConfigurationFilePut;
-import static org.ops4j.pax.exam.karaf.options.KarafDistributionOption.karafDistributionConfiguration;
-import static org.ops4j.pax.exam.karaf.options.KarafDistributionOption.keepRuntimeFolder;
-import static org.ops4j.pax.exam.karaf.options.KarafDistributionOption.logLevel;
-import static org.ops4j.pax.exam.karaf.options.KarafDistributionOption.replaceConfigurationFile;
+import static org.ops4j.pax.exam.karaf.options.KarafDistributionOption.*;
/**
* Base class for integration tests.
@@ -68,6 +69,9 @@ public abstract class BaseIT {
@Filter(timeout = 600000)
protected DefinitionsService definitionsService;
+ @Inject
+ protected BundleContext bundleContext;
+
protected void removeItems(final Class<? extends Item> ...classes) throws InterruptedException {
Condition condition = new Condition(definitionsService.getConditionType("matchAllCondition"));
for (Class<? extends Item> aClass : classes) {
@@ -206,4 +210,25 @@ public abstract class BaseIT {
}
return value;
}
+
+ protected String bundleResourceAsString(final String resourcePath) throws IOException {
+ final java.net.URL url = bundleContext.getBundle().getResource(resourcePath);
+ if (url != null) {
+ return IOUtils.toString(url);
+ } else {
+ return null;
+ }
+ }
+
+ protected String getValidatedBundleJSON(final String resourcePath, Map<String,String> parameters) throws IOException {
+ String jsonString = bundleResourceAsString(resourcePath);
+ if (parameters != null && parameters.size() > 0) {
+ for (Map.Entry<String,String> parameterEntry : parameters.entrySet()) {
+ jsonString = jsonString.replace("###" + parameterEntry.getKey() + "###", parameterEntry.getValue());
+ }
+ }
+ ObjectMapper objectMapper = CustomObjectMapper.getObjectMapper();
+ return objectMapper.writeValueAsString(objectMapper.readTree(jsonString));
+ }
+
}
diff --git a/itests/src/test/java/org/apache/unomi/itests/ContextServletIT.java b/itests/src/test/java/org/apache/unomi/itests/ContextServletIT.java
index d4cfd63..812f36c 100644
--- a/itests/src/test/java/org/apache/unomi/itests/ContextServletIT.java
+++ b/itests/src/test/java/org/apache/unomi/itests/ContextServletIT.java
@@ -21,12 +21,7 @@ import com.fasterxml.jackson.databind.ObjectMapper;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.entity.ContentType;
import org.apache.http.entity.StringEntity;
-import org.apache.unomi.api.ContextRequest;
-import org.apache.unomi.api.ContextResponse;
-import org.apache.unomi.api.Event;
-import org.apache.unomi.api.Metadata;
-import org.apache.unomi.api.Profile;
-import org.apache.unomi.api.Session;
+import org.apache.unomi.api.*;
import org.apache.unomi.api.conditions.Condition;
import org.apache.unomi.api.segments.Segment;
import org.apache.unomi.api.services.DefinitionsService;
@@ -44,6 +39,7 @@ import org.ops4j.pax.exam.spi.reactors.PerSuite;
import org.ops4j.pax.exam.util.Filter;
import javax.inject.Inject;
+import java.io.File;
import java.io.IOException;
import java.net.URI;
import java.time.LocalDateTime;
@@ -51,10 +47,11 @@ import java.time.ZoneId;
import java.time.ZoneOffset;
import java.util.Arrays;
import java.util.Date;
+import java.util.HashMap;
+import java.util.Map;
import static org.hamcrest.core.IsCollectionContaining.hasItem;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertThat;
+import static org.junit.Assert.*;
/**
@@ -346,4 +343,48 @@ public class ContextServletIT extends BaseIT {
Profile profile = this.profileService.load(profileId);
assertEquals(profileId, profile.getItemId());
}
+
+ @Test
+ public void testOGNLVulnerability() throws IOException, InterruptedException {
+
+ File vulnFile = new File("target/vuln-file.txt");
+ if (vulnFile.exists()) {
+ vulnFile.delete();
+ }
+ String vulnFileCanonicalPath = vulnFile.getCanonicalPath();
+ vulnFileCanonicalPath = vulnFileCanonicalPath.replace("\\", "\\\\"); // this is required for Windows support
+
+ Map<String,String> parameters = new HashMap<>();
+ parameters.put("VULN_FILE_PATH", vulnFileCanonicalPath);
+ HttpPost request = new HttpPost(URL + CONTEXT_URL);
+ request.setEntity(new StringEntity(getValidatedBundleJSON("security/ognl-payload-1.json", parameters), ContentType.create("application/json")));
+ TestUtils.executeContextJSONRequest(request);
+ refreshPersistence();
+ Thread.sleep(2000); //Making sure event is updated in DB
+
+ assertFalse("Vulnerability successfully executed ! File created at " + vulnFileCanonicalPath, vulnFile.exists());
+
+ }
+
+ @Test
+ public void testMVELVulnerability() throws IOException, InterruptedException {
+
+ File vulnFile = new File("target/vuln-file.txt");
+ if (vulnFile.exists()) {
+ vulnFile.delete();
+ }
+ String vulnFileCanonicalPath = vulnFile.getCanonicalPath();
+ vulnFileCanonicalPath = vulnFileCanonicalPath.replace("\\", "\\\\"); // this is required for Windows support
+
+ Map<String,String> parameters = new HashMap<>();
+ parameters.put("VULN_FILE_PATH", vulnFileCanonicalPath);
+ HttpPost request = new HttpPost(URL + CONTEXT_URL);
+ request.setEntity(new StringEntity(getValidatedBundleJSON("security/mvel-payload-1.json", parameters), ContentType.create("application/json")));
+ TestUtils.executeContextJSONRequest(request);
+ refreshPersistence();
+ Thread.sleep(2000); //Making sure event is updated in DB
+
+ assertFalse("Vulnerability successfully executed ! File created at " + vulnFileCanonicalPath, vulnFile.exists());
+
+ }
}
diff --git a/itests/src/test/resources/security/mvel-payload-1.json b/itests/src/test/resources/security/mvel-payload-1.json
new file mode 100644
index 0000000..da2c16d
--- /dev/null
+++ b/itests/src/test/resources/security/mvel-payload-1.json
@@ -0,0 +1,20 @@
+{
+ "filters": [
+ {
+ "id": "filter1",
+ "filters": [
+ {
+ "condition": {
+ "parameterValues": {
+ "propertyName": "prop",
+ "comparisonOperator": "equals",
+ "propertyValue": "script::Runtime r = Runtime.getRuntime(); r.exec(\"touch ###VULN_FILE_PATH###\");"
+ },
+ "type": "profilePropertyCondition"
+ }
+ }
+ ]
+ }
+ ],
+ "sessionId": "demo-session-id"
+}
\ No newline at end of file
diff --git a/itests/src/test/resources/security/ognl-payload-1.json b/itests/src/test/resources/security/ognl-payload-1.json
new file mode 100644
index 0000000..526a914
--- /dev/null
+++ b/itests/src/test/resources/security/ognl-payload-1.json
@@ -0,0 +1,28 @@
+{
+ "personalizations":[
+ {
+ "id":"gender-test",
+ "strategy":"matching-first",
+ "strategyOptions":{
+ "fallback":"var2"
+ },
+ "contents":[
+ {
+ "filters":[
+ {
+ "condition":{
+ "parameterValues":{
+ "propertyName":"(#runtimeclass = #this.getClass().forName(\"java.lang.Runtime\")).(#getruntimemethod = #runtimeclass.getDeclaredMethods().{^ #this.name.equals(\"getRuntime\")}[0]).(#rtobj = #getruntimemethod.invoke(null,null)).(#execmethod = #runtimeclass.getDeclaredMethods().{? #this.name.equals(\"exec\")}.{? #this.getParameters()[0].getType().getName().equals(\"java.lang.String\")}.{? #this.getParameters().length < 2}[0]).(#execmethod.invoke(#rtobj,\" touch ###VULN_FI [...]
+ "comparisonOperator":"equals",
+ "propertyValue":"male"
+ },
+ "type":"profilePropertyCondition"
+ }
+ }
+ ]
+ }
+ ]
+ }
+ ],
+ "sessionId":"demo-session-id"
+}
diff --git a/kar/pom.xml b/kar/pom.xml
index 7572c68..49f339e 100644
--- a/kar/pom.xml
+++ b/kar/pom.xml
@@ -149,6 +149,11 @@
<artifactId>unomi-web-tracker-wab</artifactId>
<version>${project.version}</version>
</dependency>
+ <dependency>
+ <groupId>org.apache.unomi</groupId>
+ <artifactId>unomi-scripting</artifactId>
+ <version>${project.version}</version>
+ </dependency>
</dependencies>
<build>
diff --git a/kar/src/main/feature/feature.xml b/kar/src/main/feature/feature.xml
index c2c15a7..dc4527b 100644
--- a/kar/src/main/feature/feature.xml
+++ b/kar/src/main/feature/feature.xml
@@ -60,6 +60,7 @@
<bundle start-level="70" start="false">mvn:org.apache.unomi/unomi-lifecycle-watcher/${project.version}</bundle>
<bundle start-level="75" start="false">mvn:org.apache.unomi/unomi-api/${project.version}</bundle>
<bundle start-level="75" start="false">mvn:org.apache.unomi/unomi-common/${project.version}</bundle>
+ <bundle start-level="75" start="false">mvn:org.apache.unomi/unomi-scripting/${project.version}</bundle>
<bundle start-level="75" start="false">mvn:org.apache.unomi/unomi-metrics/${project.version}</bundle>
<bundle start-level="75" start="false">mvn:org.apache.unomi/unomi-persistence-spi/${project.version}</bundle>
<bundle start-level="76" start="false">mvn:org.apache.unomi/unomi-persistence-elasticsearch-core/${project.version}</bundle>
diff --git a/package/src/main/resources/etc/custom.system.properties b/package/src/main/resources/etc/custom.system.properties
index 87b7464..e5b44b2 100644
--- a/package/src/main/resources/etc/custom.system.properties
+++ b/package/src/main/resources/etc/custom.system.properties
@@ -31,8 +31,13 @@ org.apache.unomi.hazelcast.network.port=${env:UNOMI_HAZELCAST_NETWORK_PORT:-5701
## Security settings ##
#######################################################################################################################
org.apache.unomi.security.root.password=${env:UNOMI_ROOT_PASSWORD:-karaf}
-org.apache.unomi.scripting.allow=${env:UNOMI_ALLOW_SCRIPTING_CLASSES:-org.apache.unomi.api.Event,org.apache.unomi.api.Profile,org.apache.unomi.api.Session,org.apache.unomi.api.Item,org.apache.unomi.api.CustomItem,ognl.*,java.lang.Object,java.util.Map,java.lang.Integer,org.mvel2.*}
+org.apache.unomi.scripting.allow=${env:UNOMI_ALLOW_SCRIPTING_CLASSES:-org.apache.unomi.api.Event,org.apache.unomi.api.Profile,org.apache.unomi.api.Session,org.apache.unomi.api.Item,org.apache.unomi.api.CustomItem,ognl.*,java.lang.Object,java.util.Map,java.util.HashMap,java.lang.Integer,org.mvel2.*}
org.apache.unomi.scripting.forbid=${env:UNOMI_FORBID_SCRIPTING_CLASSES:-}
+org.apache.unomi.scripting.filter.allow=${env:UNOMI_SCRIPTING_FILTER_ALLOW:-all}
+org.apache.unomi.scripting.filter.forbid=${env:UNOMI_SCRIPTING_FILTER_FORBID:-.*Runtime.*,.*ProcessBuilder.*,.*exec.*,.*invoke.*,.*getClass.*,.*Class.*,.*ClassLoader.*,.*System.*,.*Method.*,.*method.*,.*Compiler.*,.*Thread.*,.*FileWriter.*,.*forName.*,.*Socket.*,.*DriverManager.*}
+org.apache.unomi.security.properties.useOGNLScripting=${env:UNOMI_SCRIPTING_USE_OGNL:-false}
+org.apache.unomi.security.personalization.sanitizeConditions=${env:UNOMI_SECURITY_SANITIZEPERSONALIZATIONCONDITIONS:-true}
+
#######################################################################################################################
## HTTP Settings ##
#######################################################################################################################
diff --git a/persistence-elasticsearch/core/pom.xml b/persistence-elasticsearch/core/pom.xml
index 0aeac3e..5e31c06 100644
--- a/persistence-elasticsearch/core/pom.xml
+++ b/persistence-elasticsearch/core/pom.xml
@@ -189,10 +189,6 @@
<groupId>commons-collections</groupId>
<artifactId>commons-collections</artifactId>
</dependency>
- <dependency>
- <groupId>org.mvel</groupId>
- <artifactId>mvel2</artifactId>
- </dependency>
<dependency>
<groupId>org.apache.unomi</groupId>
@@ -218,6 +214,13 @@
<scope>provided</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.unomi</groupId>
+ <artifactId>unomi-scripting</artifactId>
+ <version>${project.version}</version>
+ <scope>provided</scope>
+ </dependency>
+
</dependencies>
<build>
diff --git a/persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/conditions/ConditionContextHelper.java b/persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/conditions/ConditionContextHelper.java
index 8355015..13bc667 100644
--- a/persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/conditions/ConditionContextHelper.java
+++ b/persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/conditions/ConditionContextHelper.java
@@ -24,25 +24,21 @@ import org.apache.logging.log4j.core.util.IOUtils;
import org.apache.lucene.analysis.charfilter.MappingCharFilterFactory;
import org.apache.lucene.analysis.util.ClasspathResourceLoader;
import org.apache.unomi.api.conditions.Condition;
-import org.apache.unomi.common.SecureFilteringClassLoader;
-import org.mvel2.MVEL;
-import org.mvel2.ParserConfiguration;
-import org.mvel2.ParserContext;
+import org.apache.unomi.scripting.ScriptExecutor;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.io.Reader;
-import java.io.Serializable;
import java.io.StringReader;
-import java.util.*;
-import java.util.concurrent.ConcurrentHashMap;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
public class ConditionContextHelper {
private static final Logger logger = LoggerFactory.getLogger(ConditionContextHelper.class);
- private static Map<String,Serializable> mvelExpressions = new ConcurrentHashMap<>();
-
private static MappingCharFilterFactory mappingCharFilterFactory;
static {
Map<String,String> args = new HashMap<>();
@@ -55,12 +51,12 @@ public class ConditionContextHelper {
}
}
- public static Condition getContextualCondition(Condition condition, Map<String, Object> context) {
+ public static Condition getContextualCondition(Condition condition, Map<String, Object> context, ScriptExecutor scriptExecutor) {
if (!hasContextualParameter(condition.getParameterValues())) {
return condition;
}
@SuppressWarnings("unchecked")
- Map<String, Object> values = (Map<String, Object>) parseParameter(context, condition.getParameterValues());
+ Map<String, Object> values = (Map<String, Object>) parseParameter(context, condition.getParameterValues(), scriptExecutor);
if (values == null) {
return null;
}
@@ -70,7 +66,7 @@ public class ConditionContextHelper {
}
@SuppressWarnings("unchecked")
- private static Object parseParameter(Map<String, Object> context, Object value) {
+ private static Object parseParameter(Map<String, Object> context, Object value, ScriptExecutor scriptExecutor) {
if (value instanceof String) {
if (((String) value).startsWith("parameter::") || ((String) value).startsWith("script::")) {
String s = (String) value;
@@ -78,13 +74,13 @@ public class ConditionContextHelper {
return context.get(StringUtils.substringAfter(s, "parameter::"));
} else if (s.startsWith("script::")) {
String script = StringUtils.substringAfter(s, "script::");
- return executeScript(context, script);
+ return scriptExecutor.execute(script, context);
}
}
} else if (value instanceof Map) {
Map<String, Object> values = new HashMap<String, Object>();
for (Map.Entry<String, Object> entry : ((Map<String, Object>) value).entrySet()) {
- Object parameter = parseParameter(context, entry.getValue());
+ Object parameter = parseParameter(context, entry.getValue(), scriptExecutor);
if (parameter == null) {
return null;
}
@@ -94,7 +90,7 @@ public class ConditionContextHelper {
} else if (value instanceof List) {
List<Object> values = new ArrayList<Object>();
for (Object o : ((List<?>) value)) {
- Object parameter = parseParameter(context, o);
+ Object parameter = parseParameter(context, o, scriptExecutor);
if (parameter != null) {
values.add(parameter);
}
@@ -104,22 +100,6 @@ public class ConditionContextHelper {
return value;
}
- private static Object executeScript(Map<String, Object> context, String script) {
- final ClassLoader tccl = Thread.currentThread().getContextClassLoader();
- try {
- if (!mvelExpressions.containsKey(script)) {
- ClassLoader secureFilteringClassLoader = new SecureFilteringClassLoader(ConditionContextHelper.class.getClassLoader());
- Thread.currentThread().setContextClassLoader(secureFilteringClassLoader);
- ParserConfiguration parserConfiguration = new ParserConfiguration();
- parserConfiguration.setClassLoader(secureFilteringClassLoader);
- mvelExpressions.put(script, MVEL.compileExpression(script, new ParserContext(parserConfiguration)));
- }
- return MVEL.executeExpression(mvelExpressions.get(script), context);
- } finally {
- Thread.currentThread().setContextClassLoader(tccl);
- }
- }
-
private static boolean hasContextualParameter(Object value) {
if (value instanceof String) {
if (((String) value).startsWith("parameter::") || ((String) value).startsWith("script::")) {
diff --git a/persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/conditions/ConditionESQueryBuilderDispatcher.java b/persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/conditions/ConditionESQueryBuilderDispatcher.java
index d08e283..56b280e 100644
--- a/persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/conditions/ConditionESQueryBuilderDispatcher.java
+++ b/persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/conditions/ConditionESQueryBuilderDispatcher.java
@@ -18,6 +18,7 @@
package org.apache.unomi.persistence.elasticsearch.conditions;
import org.apache.unomi.api.conditions.Condition;
+import org.apache.unomi.scripting.ScriptExecutor;
import org.elasticsearch.index.query.QueryBuilder;
import org.elasticsearch.index.query.QueryBuilders;
import org.slf4j.Logger;
@@ -31,10 +32,15 @@ public class ConditionESQueryBuilderDispatcher {
private static final Logger logger = LoggerFactory.getLogger(ConditionESQueryBuilderDispatcher.class.getName());
private Map<String, ConditionESQueryBuilder> queryBuilders = new ConcurrentHashMap<>();
+ private ScriptExecutor scriptExecutor;
public ConditionESQueryBuilderDispatcher() {
}
+ public void setScriptExecutor(ScriptExecutor scriptExecutor) {
+ this.scriptExecutor = scriptExecutor;
+ }
+
public void addQueryBuilder(String name, ConditionESQueryBuilder evaluator) {
queryBuilders.put(name, evaluator);
}
@@ -73,7 +79,7 @@ public class ConditionESQueryBuilderDispatcher {
if (queryBuilders.containsKey(queryBuilderKey)) {
ConditionESQueryBuilder queryBuilder = queryBuilders.get(queryBuilderKey);
- Condition contextualCondition = ConditionContextHelper.getContextualCondition(condition, context);
+ Condition contextualCondition = ConditionContextHelper.getContextualCondition(condition, context, scriptExecutor);
if (contextualCondition != null) {
return queryBuilder.buildQuery(contextualCondition, context, this);
}
@@ -107,7 +113,7 @@ public class ConditionESQueryBuilderDispatcher {
if (queryBuilders.containsKey(queryBuilderKey)) {
ConditionESQueryBuilder queryBuilder = queryBuilders.get(queryBuilderKey);
- Condition contextualCondition = ConditionContextHelper.getContextualCondition(condition, context);
+ Condition contextualCondition = ConditionContextHelper.getContextualCondition(condition, context, scriptExecutor);
if (contextualCondition != null) {
return queryBuilder.count(contextualCondition, context, this);
}
diff --git a/persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/conditions/ConditionEvaluatorDispatcher.java b/persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/conditions/ConditionEvaluatorDispatcher.java
index 6394300..cd0bb35 100644
--- a/persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/conditions/ConditionEvaluatorDispatcher.java
+++ b/persistence-elasticsearch/core/src/main/java/org/apache/unomi/persistence/elasticsearch/conditions/ConditionEvaluatorDispatcher.java
@@ -21,6 +21,7 @@ import org.apache.unomi.api.Item;
import org.apache.unomi.api.conditions.Condition;
import org.apache.unomi.metrics.MetricAdapter;
import org.apache.unomi.metrics.MetricsService;
+import org.apache.unomi.scripting.ScriptExecutor;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -37,11 +38,16 @@ public class ConditionEvaluatorDispatcher {
private Map<String, ConditionEvaluator> evaluators = new ConcurrentHashMap<>();
private MetricsService metricsService;
+ private ScriptExecutor scriptExecutor;
public void setMetricsService(MetricsService metricsService) {
this.metricsService = metricsService;
}
+ public void setScriptExecutor(ScriptExecutor scriptExecutor) {
+ this.scriptExecutor = scriptExecutor;
+ }
+
public void addEvaluator(String name, ConditionEvaluator evaluator) {
evaluators.put(name, evaluator);
}
@@ -72,7 +78,7 @@ public class ConditionEvaluatorDispatcher {
return new MetricAdapter<Boolean>(metricsService, this.getClass().getName() + ".conditions." + conditionEvaluatorKey) {
@Override
public Boolean execute(Object... args) throws Exception {
- Condition contextualCondition = ConditionContextHelper.getContextualCondition(condition, context);
+ Condition contextualCondition = ConditionContextHelper.getContextualCondition(condition, context, scriptExecutor);
if (contextualCondition != null) {
return evaluator.eval(contextualCondition, item, context, dispatcher);
} else {
diff --git a/persistence-elasticsearch/core/src/main/resources/OSGI-INF/blueprint/blueprint.xml b/persistence-elasticsearch/core/src/main/resources/OSGI-INF/blueprint/blueprint.xml
index 9cc0dda..db88c36 100644
--- a/persistence-elasticsearch/core/src/main/resources/OSGI-INF/blueprint/blueprint.xml
+++ b/persistence-elasticsearch/core/src/main/resources/OSGI-INF/blueprint/blueprint.xml
@@ -66,6 +66,7 @@
<reference id="metricsService" interface="org.apache.unomi.metrics.MetricsService" />
<reference id="hazelcastInstance" interface="com.hazelcast.core.HazelcastInstance" />
+ <reference id="scriptExecutor" interface="org.apache.unomi.scripting.ScriptExecutor" />
<service id="elasticSearchPersistenceService" ref="elasticSearchPersistenceServiceImpl">
<interfaces>
@@ -76,11 +77,13 @@
<bean id="conditionESQueryBuilderDispatcher"
class="org.apache.unomi.persistence.elasticsearch.conditions.ConditionESQueryBuilderDispatcher">
+ <property name="scriptExecutor" ref="scriptExecutor" />
</bean>
<bean id="conditionEvaluatorDispatcherImpl"
class="org.apache.unomi.persistence.elasticsearch.conditions.ConditionEvaluatorDispatcher">
<property name="metricsService" ref="metricsService" />
+ <property name="scriptExecutor" ref="scriptExecutor" />
</bean>
<bean id="elasticSearchPersistenceServiceImpl"
diff --git a/plugins/baseplugin/pom.xml b/plugins/baseplugin/pom.xml
index db94965..ac44438 100644
--- a/plugins/baseplugin/pom.xml
+++ b/plugins/baseplugin/pom.xml
@@ -84,6 +84,12 @@
<version>${project.version}</version>
<scope>provided</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.unomi</groupId>
+ <artifactId>unomi-scripting</artifactId>
+ <version>${project.version}</version>
+ <scope>provided</scope>
+ </dependency>
<!-- Unit tests -->
<dependency>
diff --git a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/PastEventConditionESQueryBuilder.java b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/PastEventConditionESQueryBuilder.java
index 65d72d1..a33f6fe 100644
--- a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/PastEventConditionESQueryBuilder.java
+++ b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/PastEventConditionESQueryBuilder.java
@@ -27,6 +27,7 @@ import org.apache.unomi.persistence.elasticsearch.conditions.ConditionESQueryBui
import org.apache.unomi.persistence.elasticsearch.conditions.ConditionESQueryBuilderDispatcher;
import org.apache.unomi.persistence.spi.PersistenceService;
import org.apache.unomi.persistence.spi.aggregate.TermsAggregate;
+import org.apache.unomi.scripting.ScriptExecutor;
import org.elasticsearch.index.query.QueryBuilder;
import org.elasticsearch.index.query.QueryBuilders;
import org.elasticsearch.index.query.RangeQueryBuilder;
@@ -38,6 +39,7 @@ public class PastEventConditionESQueryBuilder implements ConditionESQueryBuilder
private DefinitionsService definitionsService;
private PersistenceService persistenceService;
private SegmentService segmentService;
+ private ScriptExecutor scriptExecutor;
private int maximumIdsQueryCount = 5000;
private int aggregateQueryBucketSize = 5000;
@@ -50,6 +52,10 @@ public class PastEventConditionESQueryBuilder implements ConditionESQueryBuilder
this.persistenceService = persistenceService;
}
+ public void setScriptExecutor(ScriptExecutor scriptExecutor) {
+ this.scriptExecutor = scriptExecutor;
+ }
+
public void setMaximumIdsQueryCount(int maximumIdsQueryCount) {
this.maximumIdsQueryCount = maximumIdsQueryCount;
}
@@ -175,7 +181,7 @@ public class PastEventConditionESQueryBuilder implements ConditionESQueryBuilder
andCondition.setParameter("operator", "and");
andCondition.setParameter("subConditions", l);
- l.add(ConditionContextHelper.getContextualCondition(eventCondition, context));
+ l.add(ConditionContextHelper.getContextualCondition(eventCondition, context, scriptExecutor));
Integer numberOfDays = (Integer) condition.getParameter("numberOfDays");
if (numberOfDays != null) {
diff --git a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/PastEventConditionEvaluator.java b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/PastEventConditionEvaluator.java
index 8d4a6a2..0d4aaa6 100644
--- a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/PastEventConditionEvaluator.java
+++ b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/PastEventConditionEvaluator.java
@@ -26,6 +26,7 @@ import org.apache.unomi.persistence.elasticsearch.conditions.ConditionContextHel
import org.apache.unomi.persistence.elasticsearch.conditions.ConditionEvaluator;
import org.apache.unomi.persistence.elasticsearch.conditions.ConditionEvaluatorDispatcher;
import org.apache.unomi.persistence.spi.PersistenceService;
+import org.apache.unomi.scripting.ScriptExecutor;
import java.util.ArrayList;
import java.util.List;
@@ -37,6 +38,8 @@ public class PastEventConditionEvaluator implements ConditionEvaluator {
private DefinitionsService definitionsService;
+ private ScriptExecutor scriptExecutor;
+
public void setPersistenceService(PersistenceService persistenceService) {
this.persistenceService = persistenceService;
}
@@ -45,6 +48,10 @@ public class PastEventConditionEvaluator implements ConditionEvaluator {
this.definitionsService = definitionsService;
}
+ public void setScriptExecutor(ScriptExecutor scriptExecutor) {
+ this.scriptExecutor = scriptExecutor;
+ }
+
@Override
public boolean eval(Condition condition, Item item, Map<String, Object> context, ConditionEvaluatorDispatcher dispatcher) {
@@ -76,7 +83,7 @@ public class PastEventConditionEvaluator implements ConditionEvaluator {
andCondition.setParameter("operator", "and");
andCondition.setParameter("subConditions", l);
- l.add(ConditionContextHelper.getContextualCondition(eventCondition, context));
+ l.add(ConditionContextHelper.getContextualCondition(eventCondition, context, scriptExecutor));
Condition profileCondition = new Condition();
profileCondition.setConditionType(definitionsService.getConditionType("sessionPropertyCondition"));
diff --git a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/PropertyConditionEvaluator.java b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/PropertyConditionEvaluator.java
index 24466c0..b85f6f7 100644
--- a/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/PropertyConditionEvaluator.java
+++ b/plugins/baseplugin/src/main/java/org/apache/unomi/plugins/baseplugin/conditions/PropertyConditionEvaluator.java
@@ -23,7 +23,10 @@ import org.apache.commons.lang3.ObjectUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.unomi.api.*;
import org.apache.unomi.api.conditions.Condition;
-import org.apache.unomi.common.SecureFilteringClassLoader;
+import org.apache.unomi.api.rules.Rule;
+import org.apache.unomi.scripting.ExpressionFilter;
+import org.apache.unomi.scripting.ExpressionFilterFactory;
+import org.apache.unomi.scripting.SecureFilteringClassLoader;
import org.apache.unomi.persistence.elasticsearch.conditions.ConditionContextHelper;
import org.apache.unomi.persistence.elasticsearch.conditions.ConditionEvaluator;
import org.apache.unomi.persistence.elasticsearch.conditions.ConditionEvaluatorDispatcher;
@@ -52,11 +55,19 @@ public class PropertyConditionEvaluator implements ConditionEvaluator {
private Map<String, Map<String, ExpressionAccessor>> expressionCache = new HashMap<>(64);
private boolean usePropertyConditionOptimizations = true;
+ private static ClassLoader secureFilteringClassLoader = new SecureFilteringClassLoader(PropertyConditionEvaluator.class.getClassLoader());
+ private ExpressionFilterFactory expressionFilterFactory;
+
+ private boolean useOGNLScripting = Boolean.parseBoolean(System.getProperty("org.apache.unomi.security.properties.useOGNLScripting", "false"));
public void setUsePropertyConditionOptimizations(boolean usePropertyConditionOptimizations) {
this.usePropertyConditionOptimizations = usePropertyConditionOptimizations;
}
+ public void setExpressionFilterFactory(ExpressionFilterFactory expressionFilterFactory) {
+ this.expressionFilterFactory = expressionFilterFactory;
+ }
+
private int compare(Object actualValue, String expectedValue, Object expectedValueDate, Object expectedValueInteger, Object expectedValueDateExpr) {
if (expectedValue == null && expectedValueDate == null && expectedValueInteger == null && getDate(expectedValueDateExpr) == null) {
return actualValue == null ? 0 : 1;
@@ -255,7 +266,12 @@ public class PropertyConditionEvaluator implements ConditionEvaluator {
return result;
}
}
- return getOGNLPropertyValue(item, expression);
+ if (useOGNLScripting) {
+ return getOGNLPropertyValue(item, expression);
+ } else {
+ logger.warn("OGNL Off. Expression not evaluated : {}", expression);
+ return null;
+ }
}
protected Object getHardcodedPropertyValue(Item item, String expression) {
@@ -270,10 +286,16 @@ public class PropertyConditionEvaluator implements ConditionEvaluator {
return event.getTarget().getItemId();
}
if (expression.startsWith("target.properties.")) {
- if (event.getTarget() instanceof CustomItem) {
- CustomItem customItem = (CustomItem) event.getTarget();
- String expressionPart = expression.substring("target.properties.".length());
- return getNestedPropertyValue(expressionPart, customItem.getProperties());
+ String expressionPart = expression.substring("target.properties.".length());
+ Item targetItem = event.getTarget();
+ if (targetItem instanceof CustomItem) {
+ return getNestedPropertyValue(expressionPart, ((CustomItem) targetItem).getProperties());
+ } else if (targetItem instanceof Session) {
+ return getNestedPropertyValue(expressionPart, ((Session) targetItem).getProperties());
+ } else if (targetItem instanceof Rule) {
+ return null;
+ } else if (targetItem instanceof Profile) {
+ return getNestedPropertyValue(expressionPart, ((Profile) targetItem).getProperties());
}
}
if ("target.scope".equals(expression)) {
@@ -282,8 +304,41 @@ public class PropertyConditionEvaluator implements ConditionEvaluator {
if ("scope".equals(expression)) {
return event.getScope();
}
+ if ("eventType".equals(expression)) {
+ return event.getEventType();
+ }
+ if ("profile".equals(expression)) {
+ return event.getProfile();
+ }
+ if ("profileId".equals(expression)) {
+ return event.getProfileId();
+ }
+ if ("session".equals(expression)) {
+ return event.getSession();
+ }
+ if ("sessionId".equals(expression)) {
+ return event.getSessionId();
+ }
+ if ("source".equals(expression)) {
+ return event.getSource();
+ }
+ if ("target".equals(expression)) {
+ return event.getTarget();
+ }
+ if ("timeStamp".equals(expression)) {
+ return event.getTimeStamp();
+ }
+ if ("itemId".equals(expression)) {
+ return event.getItemId();
+ }
+ if ("itemType".equals(expression)) {
+ return event.getItemType();
+ }
} else if (item instanceof Session) {
Session session = (Session) item;
+ if ("scope".equals(expression)) {
+ return session.getScope();
+ }
if ("timeStamp".equals(expression)) {
return session.getTimeStamp();
}
@@ -293,12 +348,27 @@ public class PropertyConditionEvaluator implements ConditionEvaluator {
if ("size".equals(expression)) {
return session.getSize();
}
+ if ("lastEventDate".equals(expression)) {
+ return session.getLastEventDate();
+ }
if (expression.startsWith("properties.")) {
return getNestedPropertyValue(expression.substring("properties.".length()), session.getProperties());
}
if (expression.startsWith("systemProperties.")) {
return getNestedPropertyValue(expression.substring("systemProperties.".length()), session.getSystemProperties());
}
+ if ("itemId".equals(expression)) {
+ return session.getItemId();
+ }
+ if ("itemType".equals(expression)) {
+ return session.getItemType();
+ }
+ if ("profile".equals(expression)) {
+ return session.getProfile();
+ }
+ if ("profileId".equals(expression)) {
+ return session.getProfileId();
+ }
} else if (item instanceof Profile) {
Profile profile = (Profile) item;
if ("segments".equals(expression)) {
@@ -316,17 +386,38 @@ public class PropertyConditionEvaluator implements ConditionEvaluator {
if (expression.startsWith("systemProperties.")) {
return getNestedPropertyValue(expression.substring("systemProperties.".length()), profile.getSystemProperties());
}
+ if ("itemId".equals(expression)) {
+ return profile.getItemId();
+ }
+ if ("itemType".equals(expression)) {
+ return profile.getItemType();
+ }
+ if ("mergedWith".equals(expression)) {
+ return profile.getMergedWith();
+ }
} else if (item instanceof CustomItem) {
CustomItem customItem = (CustomItem) item;
if (expression.startsWith("properties.")) {
return getNestedPropertyValue(expression.substring("properties.".length()), customItem.getProperties());
}
+ if ("itemId".equals(expression)) {
+ return customItem.getItemId();
+ }
+ if ("itemType".equals(expression)) {
+ return customItem.getItemType();
+ }
+ if ("scope".equals(expression)) {
+ return customItem.getScope();
+ }
}
return NOT_OPTIMIZED_MARKER;
}
protected Object getOGNLPropertyValue(Item item, String expression) throws Exception {
- ClassLoader secureFilteringClassLoader = new SecureFilteringClassLoader(PropertyConditionEvaluator.class.getClassLoader());
+ if (expressionFilterFactory.getExpressionFilter("ognl").filter(expression) == null) {
+ logger.warn("Expression {} is not allowed !", expression);
+ return null;
+ }
OgnlContext ognlContext = getOgnlContext(secureFilteringClassLoader);
ExpressionAccessor accessor = getPropertyAccessor(item, expression, ognlContext, secureFilteringClassLoader);
return accessor != null ? accessor.get(ognlContext, item) : null;
@@ -374,8 +465,15 @@ public class PropertyConditionEvaluator implements ConditionEvaluator {
@Override
public boolean isAccessible(Map context, Object target, Member member, String propertyName) {
int modifiers = member.getModifiers();
- boolean result = Modifier.isPublic(modifiers);
- return result;
+ if (target instanceof Item) {
+ if ("getClass".equals(member.getName())) {
+ logger.warn("Target {} and member {} for property {} are not allowed by OGNL security filter", target, member, propertyName);
+ return false;
+ }
+ return Modifier.isPublic(modifiers);
+ }
+ logger.warn("Target {} and member {} for property {} are not allowed by OGNL security filter", target, member, propertyName);
+ return false;
}
}, new ClassLoaderClassResolver(classLoader),
null);
diff --git a/plugins/baseplugin/src/main/resources/META-INF/cxs/expressions/mvel.json b/plugins/baseplugin/src/main/resources/META-INF/cxs/expressions/mvel.json
new file mode 100644
index 0000000..6c0a5a0
--- /dev/null
+++ b/plugins/baseplugin/src/main/resources/META-INF/cxs/expressions/mvel.json
@@ -0,0 +1,13 @@
+[
+ "\\Q'systemProperties.goals.'+goalId+'TargetReached'\\E",
+ "\\Q'now-'+since+'d'\\E",
+ "\\Q'scores.'+scoringPlanId\\E",
+ "\\QminimumDuration*1000\\E",
+ "\\QmaximumDuration*1000\\E",
+ "\\Qprofile.properties.?nbOfVisits != null ? (profile.properties.nbOfVisits + 1) : 1\\E",
+ "\\Qsession != null ? session.size + 1 : 0\\E",
+ "\\Q'properties.optimizationTest_'+event.target.itemId\\E",
+ "\\Qevent.target.properties.variantId\\E",
+ "\\Qprofile.properties.?systemProperties.goals.\\E[\\w\\_]*\\QReached != null ? (profile.properties.systemProperties.goals.\\E[\\w\\_]*\\QReached) : 'now'\\E",
+ "\\Qprofile.properties.?systemProperties.campaigns.\\E[\\w\\_]*\\QEngaged != null ? (profile.properties.systemProperties.campaigns.\\E[\\w\\_]*\\QEngaged) : 'now'\\E"
+]
\ No newline at end of file
diff --git a/plugins/baseplugin/src/main/resources/OSGI-INF/blueprint/blueprint.xml b/plugins/baseplugin/src/main/resources/OSGI-INF/blueprint/blueprint.xml
index 5a10214..cd75ce1 100644
--- a/plugins/baseplugin/src/main/resources/OSGI-INF/blueprint/blueprint.xml
+++ b/plugins/baseplugin/src/main/resources/OSGI-INF/blueprint/blueprint.xml
@@ -44,6 +44,7 @@
<reference id="segmentService" interface="org.apache.unomi.api.services.SegmentService"/>
<reference id="eventService" interface="org.apache.unomi.api.services.EventService"/>
<reference id="configSharingService" interface="org.apache.unomi.api.services.ConfigSharingService" />
+ <reference id="scriptExecutor" interface="org.apache.unomi.scripting.ScriptExecutor"/>
<service
interface="org.apache.unomi.persistence.elasticsearch.conditions.ConditionESQueryBuilder">
@@ -102,6 +103,7 @@
<property name="definitionsService" ref="definitionsService"/>
<property name="persistenceService" ref="persistenceService"/>
<property name="segmentService" ref="segmentService"/>
+ <property name="scriptExecutor" ref="scriptExecutor" />
<property name="maximumIdsQueryCount" value="${es.maximumIdsQueryCount}"/>
<property name="aggregateQueryBucketSize" value="${es.aggregateQueryBucketSize}"/>
</bean>
@@ -163,6 +165,7 @@
<bean class="org.apache.unomi.plugins.baseplugin.conditions.PastEventConditionEvaluator">
<property name="definitionsService" ref="definitionsService"/>
<property name="persistenceService" ref="persistenceService"/>
+ <property name="scriptExecutor" ref="scriptExecutor" />
</bean>
</service>
diff --git a/plugins/baseplugin/src/test/java/org/apache/unomi/plugins/baseplugin/conditions/PropertyConditionEvaluatorTest.java b/plugins/baseplugin/src/test/java/org/apache/unomi/plugins/baseplugin/conditions/PropertyConditionEvaluatorTest.java
index 05ac126..439bd80 100644
--- a/plugins/baseplugin/src/test/java/org/apache/unomi/plugins/baseplugin/conditions/PropertyConditionEvaluatorTest.java
+++ b/plugins/baseplugin/src/test/java/org/apache/unomi/plugins/baseplugin/conditions/PropertyConditionEvaluatorTest.java
@@ -21,6 +21,9 @@ import org.apache.unomi.api.CustomItem;
import org.apache.unomi.api.Event;
import org.apache.unomi.api.Profile;
import org.apache.unomi.api.Session;
+import org.apache.unomi.scripting.ExpressionFilter;
+import org.apache.unomi.scripting.ExpressionFilterFactory;
+import org.junit.Before;
import org.junit.Test;
import java.io.File;
@@ -29,6 +32,7 @@ import java.util.concurrent.Callable;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
+import java.util.regex.Pattern;
import static junit.framework.TestCase.assertEquals;
import static junit.framework.TestCase.assertNull;
@@ -49,6 +53,27 @@ public class PropertyConditionEvaluatorTest {
private static final Profile mockProfile = generateMockProfile();
private static final Session mockSession = generateMockSession();
+ @Before
+ public void setup() {
+ propertyConditionEvaluator.setExpressionFilterFactory(new ExpressionFilterFactory() {
+ @Override
+ public ExpressionFilter getExpressionFilter(String filterCollection) {
+ Set<Pattern> allowedExpressions = new HashSet<>();
+ allowedExpressions.add(Pattern.compile("target\\.itemId"));
+ allowedExpressions.add(Pattern.compile("target\\.scope"));
+ allowedExpressions.add(Pattern.compile("target\\.properties\\.pageInfo\\.pagePath"));
+ allowedExpressions.add(Pattern.compile("target\\.properties\\.pageInfo\\.pageURL"));
+ allowedExpressions.add(Pattern.compile("size"));
+ allowedExpressions.add(Pattern.compile("lastEventDate"));
+ allowedExpressions.add(Pattern.compile("systemProperties\\.goals\\._csk6r4cgeStartReached"));
+ allowedExpressions.add(Pattern.compile("properties\\.email"));
+ allowedExpressions.add(Pattern.compile("systemProperties\\.goals\\._csk6r4cgeStartReached"));
+ Set<Pattern> forbiddenExpressions = new HashSet<>();
+ return new ExpressionFilter(allowedExpressions, forbiddenExpressions);
+ }
+ });
+ }
+
@Test
public void testHardcodedEvaluator() {
Event mockEvent = generateMockEvent();
@@ -62,7 +87,7 @@ public class PropertyConditionEvaluatorTest {
assertNull("Unexisting property should be null", propertyConditionEvaluator.getHardcodedPropertyValue(mockProfile, "properties.email"));
// here let's make sure our reporting of non optimized expressions works.
- assertEquals("Should have received the non-optimized marker string", NOT_OPTIMIZED_MARKER, propertyConditionEvaluator.getHardcodedPropertyValue(mockSession, "lastEventDate"));
+ assertEquals("Should have received the non-optimized marker string", NOT_OPTIMIZED_MARKER, propertyConditionEvaluator.getHardcodedPropertyValue(mockSession, "profile.itemId"));
}
@@ -139,6 +164,13 @@ public class PropertyConditionEvaluatorTest {
}
vulnFile = new File("target/vuln-file.txt");
assertFalse("Vulnerability successfully executed ! File created at " + vulnFileCanonicalPath, vulnFile.exists());
+ try {
+ propertyConditionEvaluator.getOGNLPropertyValue(mockEvent, "(#runtimeclass = #this.getClass().forName(\"java.lang.Runtime\")).(#getruntimemethod = #runtimeclass.getDeclaredMethods().{^ #this.name.equals(\"getRuntime\")}[0]).(#rtobj = #getruntimemethod.invoke(null,null)).(#execmethod = #runtimeclass.getDeclaredMethods().{? #this.name.equals(\"exec\")}.{? #this.getParameters()[0].getType().getName().equals(\"java.lang.String\")}.{? #this.getParameters().length < 2}[0]).(#execme [...]
+ } catch (RuntimeException | MethodFailedException re) {
+ // we ignore these exceptions as they are expected.
+ }
+ vulnFile = new File("target/vuln-file.txt");
+ assertFalse("Vulnerability successfully executed ! File created at " + vulnFileCanonicalPath, vulnFile.exists());
}
private void runHardcodedTest(int workerCount, ExecutorService executorService) throws InterruptedException {
diff --git a/pom.xml b/pom.xml
index 797d728..4600745 100644
--- a/pom.xml
+++ b/pom.xml
@@ -774,7 +774,7 @@
<dependency>
<groupId>commons-beanutils</groupId>
<artifactId>commons-beanutils</artifactId>
- <version>1.9.2</version>
+ <version>1.9.4</version>
</dependency>
<dependency>
<groupId>commons-collections</groupId>
@@ -1024,6 +1024,7 @@
<module>api</module>
<module>common</module>
<module>metrics</module>
+ <module>scripting</module>
<module>persistence-spi</module>
<module>lifecycle-watcher</module>
<module>persistence-elasticsearch</module>
diff --git a/plugins/baseplugin/pom.xml b/scripting/pom.xml
similarity index 54%
copy from plugins/baseplugin/pom.xml
copy to scripting/pom.xml
index db94965..61c10e6 100644
--- a/plugins/baseplugin/pom.xml
+++ b/scripting/pom.xml
@@ -21,83 +21,52 @@
<parent>
<groupId>org.apache.unomi</groupId>
- <artifactId>unomi-plugins</artifactId>
- <version>1.5.2-SNAPSHOT</version>
+ <artifactId>unomi-root</artifactId>
+ <version>2.0.0-SNAPSHOT</version>
</parent>
- <artifactId>unomi-plugins-base</artifactId>
- <name>Apache Unomi :: Plugins :: Base Actions and Conditions</name>
- <description>Base actions and conditions plugin for the Apache Unomi Context Server</description>
+ <artifactId>unomi-scripting</artifactId>
+ <name>Apache Unomi :: Scripting Services</name>
+ <description>Scripting services used in the Apache Unomi Context server</description>
<packaging>bundle</packaging>
<dependencies>
<dependency>
- <groupId>javax.servlet</groupId>
- <artifactId>javax.servlet-api</artifactId>
- <scope>provided</scope>
- </dependency>
- <dependency>
- <groupId>commons-collections</groupId>
- <artifactId>commons-collections</artifactId>
- </dependency>
- <dependency>
- <groupId>commons-beanutils</groupId>
- <artifactId>commons-beanutils</artifactId>
- </dependency>
- <dependency>
- <groupId>org.apache.commons</groupId>
- <artifactId>commons-lang3</artifactId>
- </dependency>
- <dependency>
- <groupId>org.apache.servicemix.bundles</groupId>
- <artifactId>org.apache.servicemix.bundles.joda-time</artifactId>
- <scope>provided</scope>
+ <groupId>junit</groupId>
+ <artifactId>junit</artifactId>
+ <scope>test</scope>
</dependency>
+
<dependency>
- <groupId>org.apache.unomi</groupId>
- <artifactId>unomi-persistence-elasticsearch-core</artifactId>
- <version>${project.version}</version>
- <scope>provided</scope>
+ <groupId>org.mvel</groupId>
+ <artifactId>mvel2</artifactId>
</dependency>
+
<dependency>
<groupId>org.apache.unomi</groupId>
- <artifactId>unomi-persistence-spi</artifactId>
- <version>${project.version}</version>
- <scope>provided</scope>
+ <artifactId>unomi-api</artifactId>
+ <version>2.0.0-SNAPSHOT</version>
+ <scope>test</scope>
</dependency>
+
<dependency>
- <groupId>ognl</groupId>
- <artifactId>ognl</artifactId>
+ <groupId>org.slf4j</groupId>
+ <artifactId>slf4j-simple</artifactId>
+ <version>1.6.6</version>
+ <scope>test</scope>
</dependency>
+
<dependency>
- <groupId>org.javassist</groupId>
- <artifactId>javassist</artifactId>
+ <groupId>org.osgi</groupId>
+ <artifactId>osgi.core</artifactId>
+ <scope>provided</scope>
</dependency>
+
<dependency>
<groupId>com.fasterxml.jackson.core</groupId>
<artifactId>jackson-databind</artifactId>
<scope>provided</scope>
</dependency>
- <dependency>
- <groupId>org.apache.unomi</groupId>
- <artifactId>unomi-common</artifactId>
- <version>${project.version}</version>
- <scope>provided</scope>
- </dependency>
-
- <!-- Unit tests -->
- <dependency>
- <groupId>junit</groupId>
- <artifactId>junit</artifactId>
- <version>4.11</version>
- <scope>test</scope>
- </dependency>
- <dependency>
- <groupId>org.slf4j</groupId>
- <artifactId>slf4j-simple</artifactId>
- <version>1.6.6</version>
- <scope>test</scope>
- </dependency>
</dependencies>
@@ -111,10 +80,9 @@
<instructions>
<Embed-Dependency>*;scope=compile|runtime</Embed-Dependency>
<Import-Package>
- jdk.internal.module;resolution:=optional,
- com.sun.tools.attach;resolution:=optional,
- javassist;resolution:=optional,
- !com.sun.jdi*,
+ org.apache.unomi.api,
+ org.apache.unomi.api.conditions,
+ org.apache.unomi.api.goals,
*
</Import-Package>
</instructions>
@@ -122,4 +90,5 @@
</plugin>
</plugins>
</build>
+
</project>
diff --git a/scripting/src/main/java/org/apache/unomi/scripting/ExpressionFilter.java b/scripting/src/main/java/org/apache/unomi/scripting/ExpressionFilter.java
new file mode 100644
index 0000000..74d4974
--- /dev/null
+++ b/scripting/src/main/java/org/apache/unomi/scripting/ExpressionFilter.java
@@ -0,0 +1,59 @@
+/*
+ * 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.unomi.scripting;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Set;
+import java.util.regex.Pattern;
+
+/**
+ * An expression filter is used to allow/deny scripts for execution.
+ */
+public class ExpressionFilter {
+ private static final Logger logger = LoggerFactory.getLogger(ExpressionFilter.class.getName());
+
+ private final Set<Pattern> allowedExpressionPatterns;
+ private final Set<Pattern> forbiddenExpressionPatterns;
+
+ public ExpressionFilter(Set<Pattern> allowedExpressionPatterns, Set<Pattern> forbiddenExpressionPatterns) {
+ this.allowedExpressionPatterns = allowedExpressionPatterns;
+ this.forbiddenExpressionPatterns = forbiddenExpressionPatterns;
+ }
+
+ public String filter(String expression) {
+ if (forbiddenExpressionPatterns != null && expressionMatches(expression, forbiddenExpressionPatterns)) {
+ logger.warn("Expression {} is forbidden by expression filter", expression);
+ return null;
+ }
+ if (allowedExpressionPatterns != null && !expressionMatches(expression, allowedExpressionPatterns)) {
+ logger.warn("Expression {} is not allowed by expression filter", expression);
+ return null;
+ }
+ return expression;
+ }
+
+ private boolean expressionMatches(String expression, Set<Pattern> patterns) {
+ for (Pattern pattern : patterns) {
+ if (pattern.matcher(expression).matches()) {
+ return true;
+ }
+ }
+ return false;
+ }
+}
diff --git a/scripting/src/main/java/org/apache/unomi/scripting/ExpressionFilterFactory.java b/scripting/src/main/java/org/apache/unomi/scripting/ExpressionFilterFactory.java
new file mode 100644
index 0000000..67bcc7a
--- /dev/null
+++ b/scripting/src/main/java/org/apache/unomi/scripting/ExpressionFilterFactory.java
@@ -0,0 +1,31 @@
+/*
+ * 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.unomi.scripting;
+
+/**
+ * A factory interface to generate ExpressionFilter instances from a centralized configuration
+ */
+public interface ExpressionFilterFactory {
+
+ /**
+ * Retrieve an expression filter for the specified filterCollection identifier.
+ * @param filterCollection a filter collection identifier, typically something like "mvel", "ognl"
+ * @return an instance of a ExpressionFilter with the configuration retrieved for the specified collection identifier
+ */
+ ExpressionFilter getExpressionFilter(String filterCollection);
+
+}
diff --git a/scripting/src/main/java/org/apache/unomi/scripting/MvelScriptExecutor.java b/scripting/src/main/java/org/apache/unomi/scripting/MvelScriptExecutor.java
new file mode 100644
index 0000000..c247c14
--- /dev/null
+++ b/scripting/src/main/java/org/apache/unomi/scripting/MvelScriptExecutor.java
@@ -0,0 +1,80 @@
+/*
+ * 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.unomi.scripting;
+
+import org.mvel2.MVEL;
+import org.mvel2.ParserConfiguration;
+import org.mvel2.ParserContext;
+
+import java.io.Serializable;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * MVEL script executor implementation
+ */
+public class MvelScriptExecutor implements ScriptExecutor {
+
+ private final static String INVALID_SCRIPT_MARKER = "--- Invalid Script Marker ---";
+
+ private Map<String, Serializable> mvelExpressions = new ConcurrentHashMap<>();
+ private SecureFilteringClassLoader secureFilteringClassLoader = new SecureFilteringClassLoader(getClass().getClassLoader());
+ private ExpressionFilterFactory expressionFilterFactory;
+
+ public void setExpressionFilterFactory(ExpressionFilterFactory expressionFilterFactory) {
+ this.expressionFilterFactory = expressionFilterFactory;
+ }
+
+ @Override
+ public Object execute(String script, Map<String, Object> context) {
+
+ final ClassLoader tccl = Thread.currentThread().getContextClassLoader();
+ try {
+ if (!mvelExpressions.containsKey(script)) {
+
+ if (expressionFilterFactory.getExpressionFilter("mvel").filter(script) == null) {
+ mvelExpressions.put(script, INVALID_SCRIPT_MARKER);
+ } else {
+ Thread.currentThread().setContextClassLoader(secureFilteringClassLoader);
+ ParserConfiguration parserConfiguration = new ParserConfiguration();
+ parserConfiguration.setClassLoader(secureFilteringClassLoader);
+ ParserContext parserContext = new ParserContext(parserConfiguration);
+
+ // override hardcoded Class Literals that are inserted by default in MVEL and that may be a security risk
+ parserContext.addImport("Runtime", String.class);
+ parserContext.addImport("System", String.class);
+ parserContext.addImport("ProcessBuilder", String.class);
+ parserContext.addImport("Class", String.class);
+ parserContext.addImport("ClassLoader", String.class);
+ parserContext.addImport("Thread", String.class);
+ parserContext.addImport("Compiler", String.class);
+ parserContext.addImport("ThreadLocal", String.class);
+ parserContext.addImport("SecurityManager", String.class);
+
+ mvelExpressions.put(script, MVEL.compileExpression(script, parserContext));
+ }
+ }
+ if (mvelExpressions.containsKey(script) && mvelExpressions.get(script) != INVALID_SCRIPT_MARKER) {
+ return MVEL.executeExpression(mvelExpressions.get(script), context);
+ } else {
+ return script;
+ }
+ } finally {
+ Thread.currentThread().setContextClassLoader(tccl);
+ }
+ }
+}
diff --git a/scripting/src/main/java/org/apache/unomi/scripting/ScriptExecutor.java b/scripting/src/main/java/org/apache/unomi/scripting/ScriptExecutor.java
new file mode 100644
index 0000000..1e8ace2
--- /dev/null
+++ b/scripting/src/main/java/org/apache/unomi/scripting/ScriptExecutor.java
@@ -0,0 +1,28 @@
+/*
+ * 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.unomi.scripting;
+
+import java.util.Map;
+
+/**
+ * An interface to execute scripts with different implementations.
+ */
+public interface ScriptExecutor {
+
+ Object execute(String script, Map<String,Object> context);
+
+}
diff --git a/common/src/main/java/org/apache/unomi/common/SecureFilteringClassLoader.java b/scripting/src/main/java/org/apache/unomi/scripting/SecureFilteringClassLoader.java
similarity index 85%
rename from common/src/main/java/org/apache/unomi/common/SecureFilteringClassLoader.java
rename to scripting/src/main/java/org/apache/unomi/scripting/SecureFilteringClassLoader.java
index 61b91bf..028d637 100644
--- a/common/src/main/java/org/apache/unomi/common/SecureFilteringClassLoader.java
+++ b/scripting/src/main/java/org/apache/unomi/scripting/SecureFilteringClassLoader.java
@@ -14,14 +14,14 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-package org.apache.unomi.common;
+package org.apache.unomi.scripting;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Set;
/**
- * A class loader that uses a whitelist and a black list of classes that it will allow to resolve. This is useful for providing proper
+ * A class loader that uses a allow list and a deny list of classes that it will allow to resolve. This is useful for providing proper
* sandboxing to scripting engine such as MVEL, OGNL or Groovy.
*/
public class SecureFilteringClassLoader extends ClassLoader {
@@ -34,7 +34,7 @@ public class SecureFilteringClassLoader extends ClassLoader {
static {
String systemAllowedClasses = System.getProperty("org.apache.unomi.scripting.allow",
- "org.apache.unomi.api.Event,org.apache.unomi.api.Profile,org.apache.unomi.api.Session,org.apache.unomi.api.Item,org.apache.unomi.api.CustomItem,ognl.*,java.lang.Object,java.util.Map,java.lang.Integer,org.mvel2.*");
+ "org.apache.unomi.api.Event,org.apache.unomi.api.Profile,org.apache.unomi.api.Session,org.apache.unomi.api.Item,org.apache.unomi.api.CustomItem,ognl.*,java.lang.Object,java.util.Map,java.util.HashMap,java.lang.Integer,org.mvel2.*");
if (systemAllowedClasses != null) {
if ("all".equals(systemAllowedClasses.trim())) {
defaultAllowedClasses = null;
@@ -98,6 +98,22 @@ public class SecureFilteringClassLoader extends ClassLoader {
return delegate.loadClass(name);
}
+ @Override
+ protected Class<?> loadClass(String name, boolean resolve) throws ClassNotFoundException {
+ if (forbiddenClasses != null && classNameMatches(forbiddenClasses, name)) {
+ throw new ClassNotFoundException("Access to class " + name + " not allowed");
+ }
+ if (allowedClasses != null && !classNameMatches(allowedClasses, name)) {
+ throw new ClassNotFoundException("Access to class " + name + " not allowed");
+ }
+ return super.loadClass(name, resolve);
+ }
+
+ @Override
+ protected Class<?> findClass(String name) throws ClassNotFoundException {
+ return super.findClass(name);
+ }
+
private boolean classNameMatches(Set<String> classesToTest, String className) {
for (String classToTest : classesToTest) {
if (classToTest.endsWith("*")) {
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
new file mode 100644
index 0000000..02cb028
--- /dev/null
+++ b/scripting/src/main/java/org/apache/unomi/scripting/internal/ExpressionFilterFactoryImpl.java
@@ -0,0 +1,190 @@
+/*
+ * 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.unomi.scripting.internal;
+
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import org.apache.unomi.scripting.ExpressionFilter;
+import org.apache.unomi.scripting.ExpressionFilterFactory;
+import org.osgi.framework.Bundle;
+import org.osgi.framework.BundleContext;
+import org.osgi.framework.BundleEvent;
+import org.osgi.framework.BundleListener;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.net.URL;
+import java.util.*;
+import java.util.regex.Pattern;
+
+public class ExpressionFilterFactoryImpl implements ExpressionFilterFactory,BundleListener {
+
+ private static final Logger logger = LoggerFactory.getLogger(ExpressionFilterFactoryImpl.class.getName());
+
+ private final Map<Bundle,Map<String,Set<Pattern>>> allowedExpressionPatternsByBundle = new HashMap<>();
+
+ private final Map<String,Set<Pattern>> allowedExpressionPatternsByCollection = new HashMap<>();
+ private final Map<String,Set<Pattern>> forbiddenExpressionPatternsByCollection = new HashMap<>();
+
+ private BundleContext bundleContext = null;
+ private final ObjectMapper objectMapper = new ObjectMapper();
+
+ private boolean expressionFiltersActivated = Boolean.parseBoolean(System.getProperty("org.apache.unomi.scripting.filter.activated", "true"));
+
+ public void setBundleContext(BundleContext bundleContext) {
+ this.bundleContext = bundleContext;
+ }
+
+ public ExpressionFilterFactoryImpl() {
+ }
+
+ public void init() {
+ String initialFilterCollections = System.getProperty("org.apache.unomi.scripting.filter.collections", "mvel,ognl");
+ 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);
+ }
+ }
+ }
+
+ if (bundleContext != null) {
+ loadPredefinedAllowedPatterns(bundleContext);
+ for (Bundle bundle : bundleContext.getBundles()) {
+ if (bundle.getBundleContext() != null && bundle.getBundleId() != bundleContext.getBundle().getBundleId()) {
+ loadPredefinedAllowedPatterns(bundle.getBundleContext());
+ }
+ }
+
+ bundleContext.addBundleListener(this);
+ }
+
+ }
+
+ public void destroy() {
+ if (bundleContext != null) {
+ bundleContext.removeBundleListener(this);
+ }
+ }
+
+ public void bundleChanged(BundleEvent event) {
+ switch (event.getType()) {
+ case BundleEvent.STARTED:
+ processBundleStartup(event.getBundle().getBundleContext());
+ break;
+ case BundleEvent.STOPPING:
+ processBundleStop(event.getBundle().getBundleContext());
+ break;
+ }
+ }
+
+ private void processBundleStartup(BundleContext bundleContext) {
+ if (bundleContext == null) {
+ return;
+ }
+ loadPredefinedAllowedPatterns(bundleContext);
+ }
+
+ private void processBundleStop(BundleContext bundleContext) {
+ if (bundleContext == null) {
+ return;
+ }
+ removePredefinedAllowedPatterns(bundleContext);
+ }
+
+ private void loadPredefinedAllowedPatterns(BundleContext bundleContext) {
+ Enumeration<URL> predefinedAllowedExpressions = bundleContext.getBundle().findEntries("META-INF/cxs/expressions", "*.json", true);
+ if (predefinedAllowedExpressions == null) {
+ return;
+ }
+
+ Map<String,Set<Pattern>> predefinedAllowedExpressionsForBundle = new HashMap<>();
+
+ while (predefinedAllowedExpressions.hasMoreElements()) {
+ URL predefinedAllowedExpressionsURL = predefinedAllowedExpressions.nextElement();
+ logger.debug("Found predefined allowed expressions at " + predefinedAllowedExpressionsURL + ", loading... ");
+ try {
+ JsonNode predefinedAllowedExpressionsNode = objectMapper.readTree(predefinedAllowedExpressionsURL);
+ Set<Pattern> bundleAllowedExpressions = new HashSet<>();
+ for (JsonNode predefinedAllowedExpressionNode : predefinedAllowedExpressionsNode) {
+ bundleAllowedExpressions.add(Pattern.compile(predefinedAllowedExpressionNode.asText()));
+ }
+ String collection = predefinedAllowedExpressionsURL.getFile().substring("/META-INF/cxs/expressions/".length(), predefinedAllowedExpressionsURL.getFile().length() - ".json".length());
+ predefinedAllowedExpressionsForBundle.put(collection, bundleAllowedExpressions);
+ Set<Pattern> existingAllowedExpressions = allowedExpressionPatternsByCollection.get(collection);
+ if (existingAllowedExpressions == null) {
+ existingAllowedExpressions = new HashSet<>();
+ }
+ existingAllowedExpressions.addAll(bundleAllowedExpressions);
+ allowedExpressionPatternsByCollection.put(collection, existingAllowedExpressions);
+ } catch (IOException e) {
+ logger.error("Error while loading expressions definition " + predefinedAllowedExpressionsURL, e);
+ }
+ }
+
+ allowedExpressionPatternsByBundle.put(bundleContext.getBundle(), predefinedAllowedExpressionsForBundle);
+ }
+
+ private void removePredefinedAllowedPatterns(BundleContext bundleContext) {
+ Map<String,Set<Pattern>> allowedExpressionPatternsForBundle = allowedExpressionPatternsByBundle.get(bundleContext.getBundle());
+ for (Map.Entry<String,Set<Pattern>> allowedExpressionPatternsEntry : allowedExpressionPatternsForBundle.entrySet()) {
+ Set<Pattern> allowedExpressionPatterns = allowedExpressionPatternsByCollection.get(allowedExpressionPatternsEntry.getKey());
+ allowedExpressionPatterns.removeAll(allowedExpressionPatternsEntry.getValue());
+ allowedExpressionPatternsByCollection.put(allowedExpressionPatternsEntry.getKey(), allowedExpressionPatterns);
+ }
+ }
+
+ @Override
+ public ExpressionFilter getExpressionFilter(String filterCollection) {
+ if (expressionFiltersActivated) {
+ return new ExpressionFilter(allowedExpressionPatternsByCollection.get(filterCollection), forbiddenExpressionPatternsByCollection.get(filterCollection));
+ } else {
+ // if expression filtering is turned off we build an expression filter with no filters and that will accept everything.
+ return new ExpressionFilter(null, null);
+ }
+ }
+}
diff --git a/scripting/src/main/resources/META-INF/cxs/expressions/mvel.json b/scripting/src/main/resources/META-INF/cxs/expressions/mvel.json
new file mode 100644
index 0000000..fe51488
--- /dev/null
+++ b/scripting/src/main/resources/META-INF/cxs/expressions/mvel.json
@@ -0,0 +1 @@
+[]
diff --git a/scripting/src/main/resources/META-INF/cxs/expressions/ognl.json b/scripting/src/main/resources/META-INF/cxs/expressions/ognl.json
new file mode 100644
index 0000000..0637a08
--- /dev/null
+++ b/scripting/src/main/resources/META-INF/cxs/expressions/ognl.json
@@ -0,0 +1 @@
+[]
\ No newline at end of file
diff --git a/scripting/src/main/resources/OSGI-INF/blueprint/blueprint.xml b/scripting/src/main/resources/OSGI-INF/blueprint/blueprint.xml
new file mode 100644
index 0000000..f4a7288
--- /dev/null
+++ b/scripting/src/main/resources/OSGI-INF/blueprint/blueprint.xml
@@ -0,0 +1,48 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+ ~ 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.
+ -->
+
+<blueprint xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xmlns:cm="http://aries.apache.org/blueprint/xmlns/blueprint-cm/v1.1.0"
+ xmlns="http://www.osgi.org/xmlns/blueprint/v1.0.0"
+ xsi:schemaLocation="http://www.osgi.org/xmlns/blueprint/v1.0.0 http://www.osgi.org/xmlns/blueprint/v1.0.0/blueprint.xsd
+ http://aries.apache.org/blueprint/xmlns/blueprint-cm/v1.1.0 http://aries.apache.org/schemas/blueprint-cm/blueprint-cm-1.1.0.xsd">
+
+ <bean id="expressionFilterFactoryImpl" init-method="init" destroy-method="destroy" class="org.apache.unomi.scripting.internal.ExpressionFilterFactoryImpl">
+ <property name="bundleContext" ref="blueprintBundleContext"/>
+ </bean>
+
+ <service id="expressionFilterFactory" ref="expressionFilterFactoryImpl">
+ <interfaces>
+ <value>org.apache.unomi.scripting.ExpressionFilterFactory</value>
+ </interfaces>
+ </service>
+
+ <bean id="mvelScriptExecutor" class="org.apache.unomi.scripting.MvelScriptExecutor">
+ <property name="expressionFilterFactory" ref="expressionFilterFactoryImpl" />
+ </bean>
+
+ <service id="scriptExecutor" ref="mvelScriptExecutor">
+ <interfaces>
+ <value>org.apache.unomi.scripting.ScriptExecutor</value>
+ </interfaces>
+ <service-properties>
+ <entry key="language" value="mvel" />
+ </service-properties>
+ </service>
+
+</blueprint>
diff --git a/scripting/src/test/java/org/apache/unomi/scripting/MvelScriptExecutorTest.java b/scripting/src/test/java/org/apache/unomi/scripting/MvelScriptExecutorTest.java
new file mode 100644
index 0000000..d843c43
--- /dev/null
+++ b/scripting/src/test/java/org/apache/unomi/scripting/MvelScriptExecutorTest.java
@@ -0,0 +1,130 @@
+/*
+ * 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.unomi.scripting;
+
+import org.apache.unomi.api.CustomItem;
+import org.apache.unomi.api.Event;
+import org.junit.Before;
+import org.junit.Test;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import java.util.regex.Pattern;
+
+import static org.junit.Assert.assertFalse;
+
+public class MvelScriptExecutorTest {
+
+ MvelScriptExecutor scriptExecutor = new MvelScriptExecutor();
+
+ public static final String MOCK_ITEM_ID = "mockItemId";
+ public static final String DIGITALL_SCOPE = "digitall";
+ public static final String PAGE_PATH_VALUE = "/site/en/home/aboutus.html";
+ public static final String PAGE_URL_VALUE = "http://localhost:8080/site/en/home/aboutus.html";
+
+ @Before
+ public void setup() {
+ scriptExecutor.setExpressionFilterFactory(new ExpressionFilterFactory() {
+ @Override
+ public ExpressionFilter getExpressionFilter(String filterCollection) {
+ Set<Pattern> allowedExpressions = new HashSet<>();
+ Set<Pattern> forbiddenExpressions = new HashSet<>();
+ return new ExpressionFilter(allowedExpressions, forbiddenExpressions);
+ }
+ });
+ }
+
+ @Test
+ public void testMVELSecurity() throws IOException {
+ Map<String, Object> ctx = new HashMap<>();
+ Event mockEvent = generateMockEvent();
+ ctx.put("event", mockEvent);
+ ctx.put("session", mockEvent.getSession());
+ ctx.put("profile", mockEvent.getProfile());
+ File vulnFile = new File("target/vuln-file.txt");
+ if (vulnFile.exists()) {
+ vulnFile.delete();
+ }
+ Object result = null;
+ try {
+ result = scriptExecutor.execute("java.io.PrintWriter writer = new java.io.PrintWriter(new java.io.BufferedWriter(new java.io.FileWriter(\"" + vulnFile.getCanonicalPath() + "\", true)));\nwriter.println(\"test\");\nwriter.close();", ctx);
+ } catch (Throwable t) {
+ // this is expected since access to these classes should not be allowed
+ System.out.println("Expected error : " + t.getMessage());
+ }
+ System.out.println("result=" + result);
+ try {
+ result = scriptExecutor.execute("import java.util.*;\nimport java.io.*;\nPrintWriter writer = new PrintWriter(new BufferedWriter(new FileWriter(\"" + vulnFile.getCanonicalPath() + "\", true)));\nwriter.println(\"test\");\nwriter.close();", ctx);
+ } catch (Throwable t) {
+ // this is expected since access to these classes should not be allowed
+ System.out.println("Expected error : " + t.getMessage());
+ }
+ System.out.println("result=" + result);
+ try {
+ result = scriptExecutor.execute("import java.util.*;\nimport java.io.*;\nnew Scanner(new File(\"" + vulnFile.getCanonicalPath() + "\")).useDelimiter(\"\\\\Z\").next();", ctx);
+ } catch (Throwable t) {
+ // this is expected since access to these classes should not be allowed
+ System.out.println("Expected error : " + t.getMessage());
+ }
+ System.out.println("result=" + result);
+ try {
+ result = scriptExecutor.execute("Runtime r = Runtime.getRuntime(); r.exec(\"touch "+vulnFile.getCanonicalPath()+"\");", ctx);
+ } catch (Throwable t) {
+ // this is expected since access to these classes should not be allowed
+ System.out.println("Expected error : " + t.getMessage());
+ }
+ System.out.println("result=" + result);
+ try {
+ result = scriptExecutor.execute("Runtime r = Runtime.getClass().forName(\"java.lang.Runtime\").getDeclaredMethod(\"getRuntime\", null ).invoke(null, null); r.exec(\"touch "+vulnFile.getCanonicalPath()+"\");", ctx);
+ } catch (Throwable t) {
+ // this is expected since access to these classes should not be allowed
+ System.out.println("Expected error : " + t.getMessage());
+ }
+ System.out.println("result=" + result);
+
+ try {
+ ctx.put("goalId", "d; " +
+ "Runtime r = Runtime.getClass().forName(\"java.lang.Runtime\").getDeclaredMethod(\"getRuntime\", null ).invoke(null, null); r.exec(\"touch " +
+ vulnFile.getCanonicalPath() +
+ "\")" +
+ " ; ");
+ result = scriptExecutor.execute("'systemProperties\\.goals\\.'+goalId+'TargetReached'", ctx);
+ } catch (Throwable t) {
+ // this is expected since access to these classes should not be allowed
+ System.out.println("Expected error : " + t.getMessage());
+ }
+ System.out.println("result=" + result);
+ assertFalse("Vulnerability successfully executed ! File created at " + vulnFile.getCanonicalPath(), vulnFile.exists());
+ }
+
+ private static Event generateMockEvent() {
+ Event mockEvent = new Event();
+ CustomItem targetItem = new CustomItem();
+ targetItem.setItemId(MOCK_ITEM_ID);
+ targetItem.setScope(DIGITALL_SCOPE);
+ mockEvent.setTarget(targetItem);
+ Map<String, Object> pageInfoMap = new HashMap<>();
+ pageInfoMap.put("pagePath", PAGE_PATH_VALUE);
+ pageInfoMap.put("pageURL", PAGE_URL_VALUE);
+ targetItem.getProperties().put("pageInfo", pageInfoMap);
+ return mockEvent;
+ }
+}
diff --git a/services/pom.xml b/services/pom.xml
index a0147d1..8dd7bbe 100644
--- a/services/pom.xml
+++ b/services/pom.xml
@@ -75,11 +75,6 @@
</dependency>
<dependency>
- <groupId>org.mvel</groupId>
- <artifactId>mvel2</artifactId>
- </dependency>
-
- <dependency>
<groupId>com.github.fge</groupId>
<artifactId>json-patch</artifactId>
<version>1.9</version>
@@ -160,6 +155,13 @@
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.unomi</groupId>
+ <artifactId>unomi-scripting</artifactId>
+ <version>${project.version}</version>
+ <scope>provided</scope>
+ </dependency>
+
</dependencies>
<build>
diff --git a/services/src/main/java/org/apache/unomi/services/actions/ActionExecutorDispatcher.java b/services/src/main/java/org/apache/unomi/services/actions/ActionExecutorDispatcher.java
index e148a94..e0c29f5 100644
--- a/services/src/main/java/org/apache/unomi/services/actions/ActionExecutorDispatcher.java
+++ b/services/src/main/java/org/apache/unomi/services/actions/ActionExecutorDispatcher.java
@@ -24,18 +24,14 @@ import org.apache.unomi.api.actions.Action;
import org.apache.unomi.api.actions.ActionDispatcher;
import org.apache.unomi.api.actions.ActionExecutor;
import org.apache.unomi.api.services.EventService;
-import org.apache.unomi.common.SecureFilteringClassLoader;
+import org.apache.unomi.scripting.ScriptExecutor;
import org.apache.unomi.metrics.MetricAdapter;
import org.apache.unomi.metrics.MetricsService;
-import org.mvel2.MVEL;
-import org.mvel2.ParserConfiguration;
-import org.mvel2.ParserContext;
import org.osgi.framework.BundleContext;
import org.osgi.framework.ServiceReference;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import java.io.Serializable;
import java.lang.reflect.InvocationTargetException;
import java.util.HashMap;
import java.util.Map;
@@ -44,12 +40,12 @@ import java.util.concurrent.ConcurrentHashMap;
public class ActionExecutorDispatcher {
private static final Logger logger = LoggerFactory.getLogger(ActionExecutorDispatcher.class.getName());
private static final String VALUE_NAME_SEPARATOR = "::";
- private final Map<String, Serializable> mvelExpressions = new ConcurrentHashMap<>();
private final Map<String, ValueExtractor> valueExtractors = new HashMap<>(11);
private Map<String, ActionExecutor> executors = new ConcurrentHashMap<>();
private MetricsService metricsService;
private Map<String, ActionDispatcher> actionDispatchers = new ConcurrentHashMap<>();
private BundleContext bundleContext;
+ private ScriptExecutor scriptExecutor;
public void setMetricsService(MetricsService metricsService) {
this.metricsService = metricsService;
@@ -59,6 +55,10 @@ public class ActionExecutorDispatcher {
this.bundleContext = bundleContext;
}
+ public void setScriptExecutor(ScriptExecutor scriptExecutor) {
+ this.scriptExecutor = scriptExecutor;
+ }
+
public ActionExecutorDispatcher() {
valueExtractors.put("profileProperty", new ValueExtractor() {
@Override
@@ -226,24 +226,12 @@ public class ActionExecutorDispatcher {
}
}
- protected Object executeScript(String valueAsString, Event event) {
- final ClassLoader tccl = Thread.currentThread().getContextClassLoader();
- try {
- ClassLoader secureFilteringClassLoader = new SecureFilteringClassLoader(getClass().getClassLoader());
- Thread.currentThread().setContextClassLoader(secureFilteringClassLoader);
- if (!mvelExpressions.containsKey(valueAsString)) {
- ParserConfiguration parserConfiguration = new ParserConfiguration();
- parserConfiguration.setClassLoader(secureFilteringClassLoader);
- mvelExpressions.put(valueAsString, MVEL.compileExpression(valueAsString, new ParserContext(parserConfiguration)));
- }
- Map<String, Object> ctx = new HashMap<>();
- ctx.put("event", event);
- ctx.put("session", event.getSession());
- ctx.put("profile", event.getProfile());
- return MVEL.executeExpression(mvelExpressions.get(valueAsString), ctx);
- } finally {
- Thread.currentThread().setContextClassLoader(tccl);
- }
+ protected Object executeScript(String script, Event event) {
+ Map<String, Object> context = new HashMap<>();
+ context.put("event", event);
+ context.put("session", event.getSession());
+ context.put("profile", event.getProfile());
+ return scriptExecutor.execute(script, context);
}
}
diff --git a/services/src/main/resources/OSGI-INF/blueprint/blueprint.xml b/services/src/main/resources/OSGI-INF/blueprint/blueprint.xml
index 72cf5c0..b2cf0c0 100644
--- a/services/src/main/resources/OSGI-INF/blueprint/blueprint.xml
+++ b/services/src/main/resources/OSGI-INF/blueprint/blueprint.xml
@@ -64,7 +64,8 @@
<reference id="karafCellarEventProducer" interface="org.apache.karaf.cellar.core.event.EventProducer" />
<reference id="karafCellarGroupManager" interface="org.apache.karaf.cellar.core.GroupManager" />
<reference id="osgiConfigurationAdmin" interface="org.osgi.service.cm.ConfigurationAdmin"/>
- <reference id="metricsService" interface="org.apache.unomi.metrics.MetricsService" />
+ <reference id="metricsService" interface="org.apache.unomi.metrics.MetricsService"/>
+ <reference id="scriptExecutor" interface="org.apache.unomi.scripting.ScriptExecutor" />
<!-- Service definitions -->
@@ -136,6 +137,7 @@
<bean id="actionExecutorDispatcherImpl"
class="org.apache.unomi.services.actions.ActionExecutorDispatcher">
<property name="metricsService" ref="metricsService" />
+ <property name="scriptExecutor" ref="scriptExecutor"/>
<property name="bundleContext" ref="blueprintBundleContext"/>
</bean>
diff --git a/services/src/test/java/org/apache/unomi/services/actions/ActionExecutorDispatcherTest.java b/services/src/test/java/org/apache/unomi/services/actions/ActionExecutorDispatcherTest.java
deleted file mode 100644
index 813c067..0000000
--- a/services/src/test/java/org/apache/unomi/services/actions/ActionExecutorDispatcherTest.java
+++ /dev/null
@@ -1,106 +0,0 @@
-/*
- * 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.unomi.services.actions;
-
-import org.apache.unomi.api.CustomItem;
-import org.apache.unomi.api.Event;
-import org.apache.unomi.common.SecureFilteringClassLoader;
-import org.junit.Test;
-import org.mvel2.CompileException;
-import org.mvel2.MVEL;
-import org.mvel2.ParserConfiguration;
-import org.mvel2.ParserContext;
-
-import java.io.*;
-import java.util.*;
-
-import static org.junit.Assert.assertFalse;
-
-public class ActionExecutorDispatcherTest {
-
- public static final String MOCK_ITEM_ID = "mockItemId";
- public static final String DIGITALL_SCOPE = "digitall";
- public static final String PAGE_PATH_VALUE = "/site/en/home/aboutus.html";
- public static final String PAGE_URL_VALUE = "http://localhost:8080/site/en/home/aboutus.html";
-
- @Test
- public void testMVELSecurity() throws IOException {
- Map<String, Object> ctx = new HashMap<>();
- Event mockEvent = generateMockEvent();
- ctx.put("event", mockEvent);
- ctx.put("session", mockEvent.getSession());
- ctx.put("profile", mockEvent.getProfile());
- File vulnFile = new File("target/vuln-file.txt");
- if (vulnFile.exists()) {
- vulnFile.delete();
- }
- Object result = null;
- try {
- result = executeMVEL("java.io.PrintWriter writer = new java.io.PrintWriter(new java.io.BufferedWriter(new java.io.FileWriter(\"" + vulnFile.getCanonicalPath() + "\", true)));\nwriter.println(\"test\");\nwriter.close();", ctx);
- } catch (CompileException ce) {
- // this is expected since access to these classes should not be allowed
- }
- System.out.println("result=" + result);
- try {
- result = executeMVEL("import java.util.*;\nimport java.io.*;\nPrintWriter writer = new PrintWriter(new BufferedWriter(new FileWriter(\"" + vulnFile.getCanonicalPath() + "\", true)));\nwriter.println(\"test\");\nwriter.close();", ctx);
- } catch (CompileException ce) {
- // this is expected since access to these classes should not be allowed
- }
- System.out.println("result=" + result);
- try {
- result = executeMVEL("import java.util.*;\nimport java.io.*;\nnew Scanner(new File(\"" + vulnFile.getCanonicalPath() + "\")).useDelimiter(\"\\\\Z\").next();", ctx);
- } catch (CompileException ce) {
- // this is expected since access to these classes should not be allowed
- }
- System.out.println("result=" + result);
- assertFalse("Vulnerability successfully executed ! File created at " + vulnFile.getCanonicalPath(), vulnFile.exists());
- }
-
- private Object executeMVEL(String expression, Object ctx) {
- final ClassLoader tccl = Thread.currentThread().getContextClassLoader();
- try {
- ParserConfiguration parserConfiguration = new ParserConfiguration();
- ClassLoader secureFilteringClassLoader = new SecureFilteringClassLoader(getClass().getClassLoader());
- Thread.currentThread().setContextClassLoader(secureFilteringClassLoader);
- parserConfiguration.setClassLoader(secureFilteringClassLoader);
- ParserContext parserContext = new ParserContext(parserConfiguration);
- Serializable compiledExpression = MVEL.compileExpression(expression, parserContext);
- try {
- return MVEL.executeExpression(compiledExpression, ctx, new HashMap());
- } catch (CompileException ce) {
- // this is expected
- }
- return null;
- } finally {
- Thread.currentThread().setContextClassLoader(tccl);
- }
- }
-
- private static Event generateMockEvent() {
- Event mockEvent = new Event();
- CustomItem targetItem = new CustomItem();
- targetItem.setItemId(MOCK_ITEM_ID);
- targetItem.setScope(DIGITALL_SCOPE);
- mockEvent.setTarget(targetItem);
- Map<String, Object> pageInfoMap = new HashMap<>();
- pageInfoMap.put("pagePath", PAGE_PATH_VALUE);
- pageInfoMap.put("pageURL", PAGE_URL_VALUE);
- targetItem.getProperties().put("pageInfo", pageInfoMap);
- return mockEvent;
- }
-
-}
diff --git a/tools/shell-commands/src/main/resources/OSGI-INF/blueprint/blueprint.xml b/tools/shell-commands/src/main/resources/OSGI-INF/blueprint/blueprint.xml
index d490a0f..0d0c7bd 100644
--- a/tools/shell-commands/src/main/resources/OSGI-INF/blueprint/blueprint.xml
+++ b/tools/shell-commands/src/main/resources/OSGI-INF/blueprint/blueprint.xml
@@ -33,6 +33,7 @@
<value>org.apache.unomi.lifecycle-watcher</value>
<value>org.apache.unomi.api</value>
<value>org.apache.unomi.common</value>
+ <value>org.apache.unomi.scripting</value>
<value>org.apache.unomi.metrics</value>
<value>org.apache.unomi.persistence-spi</value>
<value>org.apache.unomi.persistence-elasticsearch-core</value>
diff --git a/wab/src/main/java/org/apache/unomi/web/ContextServlet.java b/wab/src/main/java/org/apache/unomi/web/ContextServlet.java
index 1557f5a..ae163c2 100644
--- a/wab/src/main/java/org/apache/unomi/web/ContextServlet.java
+++ b/wab/src/main/java/org/apache/unomi/web/ContextServlet.java
@@ -22,6 +22,7 @@ import com.fasterxml.jackson.databind.ObjectMapper;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.unomi.api.*;
+import org.apache.unomi.api.conditions.Condition;
import org.apache.unomi.api.services.*;
import org.apache.unomi.persistence.spi.CustomObjectMapper;
import org.slf4j.Logger;
@@ -35,7 +36,6 @@ import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import java.io.IOException;
-import java.io.InputStream;
import java.io.Writer;
import java.util.*;
@@ -59,6 +59,8 @@ public class ContextServlet extends HttpServlet {
private PersonalizationService personalizationService;
private ConfigSharingService configSharingService;
+ private boolean sanitizeConditions = Boolean.parseBoolean(System.getProperty("org.apache.unomi.security.personalization.sanitizeConditions", "true"));
+
@Override
public void init(ServletConfig config) throws ServletException {
super.init(config);
@@ -363,7 +365,7 @@ public class ContextServlet extends HttpServlet {
List<PersonalizationService.PersonalizedContent> filterNodes = contextRequest.getFilters();
if (filterNodes != null) {
data.setFilteringResults(new HashMap<>());
- for (PersonalizationService.PersonalizedContent personalizedContent : filterNodes) {
+ for (PersonalizationService.PersonalizedContent personalizedContent : sanitizePersonalizedContentObjects(filterNodes)) {
data.getFilteringResults().put(personalizedContent.getId(), personalizationService.filter(profile,
session, personalizedContent));
}
@@ -372,7 +374,7 @@ public class ContextServlet extends HttpServlet {
List<PersonalizationService.PersonalizationRequest> personalizations = contextRequest.getPersonalizations();
if (personalizations != null) {
data.setPersonalizations(new HashMap<>());
- for (PersonalizationService.PersonalizationRequest personalization : personalizations) {
+ for (PersonalizationService.PersonalizationRequest personalization : sanitizePersonalizations(personalizations)) {
data.getPersonalizations().put(personalization.getId(), personalizationService.personalizeList(profile,
session, personalization));
}
@@ -470,4 +472,89 @@ public class ContextServlet extends HttpServlet {
public void setConfigSharingService(ConfigSharingService configSharingService) {
this.configSharingService = configSharingService;
}
+
+ private List<PersonalizationService.PersonalizedContent> sanitizePersonalizedContentObjects(List<PersonalizationService.PersonalizedContent> personalizedContentObjects) {
+ if (!sanitizeConditions) {
+ return personalizedContentObjects;
+ }
+ List<PersonalizationService.PersonalizedContent> result = new ArrayList<>();
+ for (PersonalizationService.PersonalizedContent personalizedContentObject : personalizedContentObjects) {
+ boolean foundInvalidCondition = false;
+ if (personalizedContentObject.getFilters() != null) {
+ for (PersonalizationService.Filter filter : personalizedContentObject.getFilters()) {
+ if (sanitizeCondition(filter.getCondition()) == null) {
+ foundInvalidCondition = true;
+ break;
+ }
+ }
+ }
+ if (!foundInvalidCondition) {
+ result.add(personalizedContentObject);
+ }
+ }
+
+ return result;
+ }
+
+ private List<PersonalizationService.PersonalizationRequest> sanitizePersonalizations(List<PersonalizationService.PersonalizationRequest> personalizations) {
+ if (!sanitizeConditions) {
+ return personalizations;
+ }
+ List<PersonalizationService.PersonalizationRequest> result = new ArrayList<>();
+ for (PersonalizationService.PersonalizationRequest personalizationRequest : personalizations) {
+ List<PersonalizationService.PersonalizedContent> personalizedContents = sanitizePersonalizedContentObjects(personalizationRequest.getContents());
+ if (personalizedContents != null && personalizedContents.size() > 0) {
+ result.add(personalizationRequest);
+ }
+ }
+ return result;
+ }
+
+ private Condition sanitizeCondition(Condition condition) {
+ Map<String,Object> newParameterValues = new LinkedHashMap<>();
+ for (Map.Entry<String,Object> parameterEntry : condition.getParameterValues().entrySet()) {
+ Object sanitizedValue = sanitizeValue(parameterEntry.getValue());
+ if (sanitizedValue != null) {
+ newParameterValues.put(parameterEntry.getKey(), parameterEntry.getValue());
+ } else {
+ return null;
+ }
+ }
+ return condition;
+ }
+
+ private Object sanitizeValue(Object value) {
+ if (value instanceof String) {
+ String stringValue = (String) value;
+ if (stringValue.startsWith("script::") || stringValue.startsWith("parameter::")) {
+ logger.warn("Scripting detected in context request with value {}, filtering out...", value);
+ return null;
+ } else {
+ return stringValue;
+ }
+ } else if (value instanceof List) {
+ List values = (List) value;
+ List newValues = new ArrayList();
+ for (Object listObject : values) {
+ Object newObject = sanitizeValue(listObject);
+ if (newObject != null) {
+ newValues.add(newObject);
+ }
+ }
+ return values;
+ } else if (value instanceof Map) {
+ Map<Object,Object> newMap = new LinkedHashMap<>();
+ ((Map<?, ?>) value).forEach((key, value1) -> {
+ Object newObject = sanitizeValue(value1);
+ if (newObject != null) {
+ newMap.put(key, newObject);
+ }
+ });
+ return newMap;
+ } else if (value instanceof Condition) {
+ return sanitizeCondition((Condition) value);
+ } else {
+ return value;
+ }
+ }
}