This is an automated email from the ASF dual-hosted git repository.
shuber pushed a commit to branch unomi-1.4.x
in repository https://gitbox.apache.org/repos/asf/unomi.git
The following commit(s) were added to refs/heads/unomi-1.4.x by this push:
new 3d15022 Make it possible to white list and black list classes used in
OGNL and MVEL2 scripting languages (#158)
3d15022 is described below
commit 3d15022e4b52a2fcc0912ef6c259c3905d4f374a
Author: Serge Huber <[email protected]>
AuthorDate: Tue May 12 10:00:23 2020 +0200
Make it possible to white list and black list classes used in OGNL and
MVEL2 scripting languages (#158)
* Make it possible to white list and black list classes used in OGNL and
MVEL2 scripting languages
* Add new integration test to AllITs list
* Add some documentation for the configuration parameters
(cherry picked from commit 789ae8e820c507866b9c91590feebffa4e996f5e)
---
.../unomi/common/SecureFilteringClassLoader.java | 112 ++++++++++++++++
.../test/java/org/apache/unomi/itests/AllITs.java | 5 +-
.../java/org/apache/unomi/itests/SecurityIT.java | 122 +++++++++++++++++
.../java/org/apache/unomi/itests/TestUtils.java | 82 +++++++++---
manual/src/main/asciidoc/configuration.adoc | 18 +++
.../main/resources/etc/custom.system.properties | 3 +-
.../main/resources/etc/org.ops4j.pax.url.mvn.cfg | 146 +++++++++++++++++++++
persistence-elasticsearch/core/pom.xml | 6 +
.../conditions/ConditionContextHelper.java | 29 ++--
plugins/baseplugin/pom.xml | 6 +
.../conditions/PropertyConditionEvaluator.java | 58 ++++++--
.../conditions/PropertyConditionEvaluatorTest.java | 44 ++++++-
pom.xml | 2 +-
services/pom.xml | 7 +
.../services/actions/ActionExecutorDispatcher.java | 39 +++---
.../actions/ActionExecutorDispatcherTest.java | 106 +++++++++++++++
16 files changed, 720 insertions(+), 65 deletions(-)
diff --git
a/common/src/main/java/org/apache/unomi/common/SecureFilteringClassLoader.java
b/common/src/main/java/org/apache/unomi/common/SecureFilteringClassLoader.java
new file mode 100644
index 0000000..61b91bf
--- /dev/null
+++
b/common/src/main/java/org/apache/unomi/common/SecureFilteringClassLoader.java
@@ -0,0 +1,112 @@
+/*
+ * 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.common;
+
+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
+ * sandboxing to scripting engine such as MVEL, OGNL or Groovy.
+ */
+public class SecureFilteringClassLoader extends ClassLoader {
+
+ private Set<String> allowedClasses = null;
+ private Set<String> forbiddenClasses = null;
+
+ private static Set<String> defaultAllowedClasses = null;
+ private static Set<String> defaultForbiddenClasses = null;
+
+ 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.*");
+ if (systemAllowedClasses != null) {
+ if ("all".equals(systemAllowedClasses.trim())) {
+ defaultAllowedClasses = null;
+ } else {
+ if (systemAllowedClasses.trim().length() > 0) {
+ String[] systemAllowedClassesParts =
systemAllowedClasses.split(",");
+ defaultAllowedClasses = new HashSet<>();
+
defaultAllowedClasses.addAll(Arrays.asList(systemAllowedClassesParts));
+ } else {
+ defaultAllowedClasses = null;
+ }
+ }
+ }
+
+ String systemForbiddenClasses =
System.getProperty("org.apache.unomi.scripting.forbid", null);
+ if (systemForbiddenClasses != null) {
+ if (systemForbiddenClasses.trim().length() > 0) {
+ String[] systemForbiddenClassesParts =
systemForbiddenClasses.split(",");
+ defaultForbiddenClasses = new HashSet<>();
+
defaultForbiddenClasses.addAll(Arrays.asList(systemForbiddenClassesParts));
+ } else {
+ defaultForbiddenClasses = null;
+ }
+ }
+
+ }
+
+ ClassLoader delegate;
+
+ /**
+ * Sets up the securing filtering class loader, using the default allowed
and forbidden classes. By default the
+ * @param delegate the class loader we delegate to if the filtering was
not applied.
+ */
+ public SecureFilteringClassLoader(ClassLoader delegate) {
+ this(defaultAllowedClasses, defaultForbiddenClasses, delegate);
+ }
+
+ /**
+ * Sets up the secure filtering class loader
+ * @param allowedClasses the list of allowed FQN class names, or if this
filtering is to be deactivated, pass null.
+ * if you want to allow no class, pass an empty
hashset
+ * @param forbiddenClasses the list of forbidden FQN class names, or if
this filtering is to be deactivated, pass null or an empty set
+ *
+ * @param delegate the class loader we delegate to if the filtering was
not applied.
+ */
+ public SecureFilteringClassLoader(Set<String> allowedClasses, Set<String>
forbiddenClasses, ClassLoader delegate) {
+ super(delegate);
+ this.allowedClasses = allowedClasses;
+ this.forbiddenClasses = forbiddenClasses;
+ this.delegate = delegate;
+ }
+
+ @Override
+ public Class<?> loadClass(String name) 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 delegate.loadClass(name);
+ }
+
+ private boolean classNameMatches(Set<String> classesToTest, String
className) {
+ for (String classToTest : classesToTest) {
+ if (classToTest.endsWith("*")) {
+ if (className.startsWith(classToTest.substring(0,
classToTest.length() - 1))) return true;
+ } else {
+ if (className.equals(classToTest)) return true;
+ }
+ }
+ return false;
+ }
+
+}
diff --git a/itests/src/test/java/org/apache/unomi/itests/AllITs.java
b/itests/src/test/java/org/apache/unomi/itests/AllITs.java
index 5456a30..42d4b7d 100644
--- a/itests/src/test/java/org/apache/unomi/itests/AllITs.java
+++ b/itests/src/test/java/org/apache/unomi/itests/AllITs.java
@@ -23,7 +23,7 @@ import org.junit.runners.Suite.SuiteClasses;
/**
* Defines suite of test classes to run.
- *
+ *
* @author Sergiy Shyrkov
*/
@RunWith(Suite.class)
@@ -40,7 +40,8 @@ import org.junit.runners.Suite.SuiteClasses;
ProfileExportIT.class,
PropertiesUpdateActionIT.class,
ModifyConsentIT.class,
- PatchIT.class
+ PatchIT.class,
+ SecurityIT.class
})
public class AllITs {
}
diff --git a/itests/src/test/java/org/apache/unomi/itests/SecurityIT.java
b/itests/src/test/java/org/apache/unomi/itests/SecurityIT.java
new file mode 100644
index 0000000..c2d5a3f
--- /dev/null
+++ b/itests/src/test/java/org/apache/unomi/itests/SecurityIT.java
@@ -0,0 +1,122 @@
+/*
+ * 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.itests;
+
+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.conditions.Condition;
+import org.apache.unomi.api.services.PersonalizationService;
+import org.apache.unomi.lifecycle.BundleWatcher;
+import org.apache.unomi.persistence.spi.CustomObjectMapper;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.ops4j.pax.exam.junit.PaxExam;
+import org.ops4j.pax.exam.spi.reactors.ExamReactorStrategy;
+import org.ops4j.pax.exam.spi.reactors.PerSuite;
+import org.ops4j.pax.exam.util.Filter;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import javax.inject.Inject;
+import java.io.File;
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static org.junit.Assert.assertFalse;
+
+@RunWith(PaxExam.class)
+@ExamReactorStrategy(PerSuite.class)
+public class SecurityIT extends BaseIT {
+
+ private final static Logger LOGGER =
LoggerFactory.getLogger(SecurityIT.class);
+
+ private static final String SESSION_ID = "vuln-session-id";
+
+ private ObjectMapper objectMapper;
+ private TestUtils testUtils = new TestUtils();
+
+ @Inject
+ @Filter(timeout = 600000)
+ protected BundleWatcher bundleWatcher;
+
+ @Before
+ public void setUp() throws InterruptedException {
+ while (!bundleWatcher.isStartupComplete()) {
+ LOGGER.info("Waiting for startup to complete...");
+ Thread.sleep(1000);
+ }
+ objectMapper = CustomObjectMapper.getObjectMapper();
+ }
+
+ @Test
+ public void testOGNLInjection() throws IOException, InterruptedException {
+ ContextRequest contextRequest = new ContextRequest();
+ List<PersonalizationService.PersonalizationRequest> personalizations =
new ArrayList<>();
+ PersonalizationService.PersonalizationRequest personalizationRequest =
new PersonalizationService.PersonalizationRequest();
+ personalizationRequest.setId("vuln-test");
+ personalizationRequest.setStrategy("matching-first");
+ Map<String,Object> strategyOptions = new HashMap<>();
+ strategyOptions.put("fallback", "var2");
+ personalizationRequest.setStrategyOptions(strategyOptions);
+ List<PersonalizationService.PersonalizedContent>
personalizationContents = new ArrayList<>();
+ PersonalizationService.PersonalizedContent var1Content = new
PersonalizationService.PersonalizedContent();
+ var1Content.setId("var1");
+ List<PersonalizationService.Filter> filters = new ArrayList<>();
+ PersonalizationService.Filter filter = new
PersonalizationService.Filter();
+ Condition condition = new Condition();
+ File vulnFile = new File("target/vuln-file.txt");
+ if (vulnFile.exists()) {
+ vulnFile.delete();
+ }
+ condition.setConditionTypeId("profilePropertyCondition");
+ condition.setParameter("propertyName",
"@java.lang.Runtime@getRuntime().exec('touch " + vulnFile.getCanonicalPath() +
"')");
+ condition.setParameter("comparisonOperator", "equals");
+ condition.setParameter("propertyValue" , "script::java.io.PrintWriter
writer = new java.io.PrintWriter(new java.io.BufferedWriter(new
java.io.FileWriter(\"" + vulnFile.getCanonicalPath() + "\",
true)));\nwriter.println(\"test\");\nwriter.close();");
+ filter.setCondition(condition);
+ filters.add(filter);
+ var1Content.setFilters(filters);
+ personalizationContents.add(var1Content);
+ PersonalizationService.PersonalizedContent var2Content = new
PersonalizationService.PersonalizedContent();
+ var2Content.setId("var2");
+ personalizationContents.add(var2Content);
+ personalizationRequest.setContents(personalizationContents);
+
+ personalizations.add(personalizationRequest);
+ contextRequest.setPersonalizations(personalizations);
+
+ contextRequest.setSessionId(SESSION_ID);
+ HttpPost request = new HttpPost(URL + "/context.json");
+ request.setEntity(new
StringEntity(objectMapper.writeValueAsString(contextRequest),
ContentType.create("application/json")));
+
+ TestUtils.RequestResponse response =
executeContextJSONRequest(request, SESSION_ID);
+ assertFalse("Vulnerability successfully executed ! File created at " +
vulnFile.getCanonicalPath(), vulnFile.exists());
+ }
+
+ private TestUtils.RequestResponse executeContextJSONRequest(HttpPost
request, String sessionId) throws IOException {
+ return testUtils.executeContextJSONRequest(request, sessionId);
+ }
+
+
+
+}
diff --git a/itests/src/test/java/org/apache/unomi/itests/TestUtils.java
b/itests/src/test/java/org/apache/unomi/itests/TestUtils.java
index 7cc9733..066adbd 100644
--- a/itests/src/test/java/org/apache/unomi/itests/TestUtils.java
+++ b/itests/src/test/java/org/apache/unomi/itests/TestUtils.java
@@ -19,29 +19,75 @@ package org.apache.unomi.itests;
import com.fasterxml.jackson.databind.ObjectMapper;
import org.apache.http.HttpResponse;
+import org.apache.http.client.methods.CloseableHttpResponse;
+import org.apache.http.client.methods.HttpPost;
+import org.apache.http.entity.ContentType;
+import org.apache.http.impl.client.HttpClientBuilder;
import org.apache.http.util.EntityUtils;
+import org.apache.unomi.api.ContextResponse;
import org.apache.unomi.persistence.spi.CustomObjectMapper;
+import org.junit.Assert;
import java.io.IOException;
public class TestUtils {
+ private static final String JSON_MYME_TYPE = "application/json";
- public static <T> T retrieveResourceFromResponse(HttpResponse response,
Class<T> clazz) throws IOException {
- if (response == null) {
- return null;
- }
- if (response.getEntity() == null) {
- return null;
- }
- String jsonFromResponse = EntityUtils.toString(response.getEntity());
- // ObjectMapper mapper = new
ObjectMapper().configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES,
false);
- ObjectMapper mapper = CustomObjectMapper.getObjectMapper();
- try {
- T value = mapper.readValue(jsonFromResponse, clazz);
- return value;
- } catch (Throwable t) {
- t.printStackTrace();
- }
- return null;
- }
+ public static <T> T retrieveResourceFromResponse(HttpResponse response,
Class<T> clazz) throws IOException {
+ if (response == null) {
+ return null;
+ }
+ if (response.getEntity() == null) {
+ return null;
+ }
+ String jsonFromResponse =
EntityUtils.toString(response.getEntity());
+ // ObjectMapper mapper = new
ObjectMapper().configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES,
false);
+ ObjectMapper mapper = CustomObjectMapper.getObjectMapper();
+ try {
+ T value = mapper.readValue(jsonFromResponse, clazz);
+ return value;
+ } catch (Throwable t) {
+ t.printStackTrace();
+ }
+ return null;
+ }
+
+ public RequestResponse executeContextJSONRequest(HttpPost request,
String sessionId) throws IOException {
+ try (CloseableHttpResponse response =
HttpClientBuilder.create().build().execute(request)) {
+ // validate mimeType
+ String mimeType =
ContentType.getOrDefault(response.getEntity()).getMimeType();
+ Assert.assertEquals("Response content type should be "
+ JSON_MYME_TYPE, JSON_MYME_TYPE, mimeType);
+
+ // validate context
+ ContextResponse context =
TestUtils.retrieveResourceFromResponse(response, ContextResponse.class);
+ Assert.assertNotNull("Context should not be null",
context);
+ Assert.assertNotNull("Context profileId should not be
null", context.getProfileId());
+ Assert.assertEquals("Context sessionId should be the
same as the sessionId used to request the context", sessionId,
+ context.getSessionId());
+
+ String cookieHeader = null;
+ if (response.containsHeader("Set-Cookie")) {
+ cookieHeader =
response.getHeaders("Set-Cookie")[0].toString().substring(12);
+ }
+ return new RequestResponse(context, cookieHeader);
+ }
+ }
+
+ public static class RequestResponse {
+ private ContextResponse contextResponse;
+ private String cookieHeaderValue;
+
+ public RequestResponse(ContextResponse contextResponse, String
cookieHeaderValue) {
+ this.contextResponse = contextResponse;
+ this.cookieHeaderValue = cookieHeaderValue;
+ }
+
+ public ContextResponse getContextResponse() {
+ return contextResponse;
+ }
+
+ public String getCookieHeaderValue() {
+ return cookieHeaderValue;
+ }
+ }
}
diff --git a/manual/src/main/asciidoc/configuration.adoc
b/manual/src/main/asciidoc/configuration.adoc
index 81a04d0..4e65016 100644
--- a/manual/src/main/asciidoc/configuration.adoc
+++ b/manual/src/main/asciidoc/configuration.adoc
@@ -193,6 +193,24 @@ You should now have SSL setup on Karaf with your
certificate, and you can test i
Changing the default Karaf password can be done by modifying the
`org.apache.unomi.security.root.password` in the
`$MY_KARAF_HOME/etc/unomi.custom.system.properties` file
+==== Scripting security
+
+By default, scripting (using in conditions, segments and rules) is controlled
by a custom classloader that is quite
+restrictive and using a white-list/black list system. It is controlled through
the following property in the
+`unomi.custom.system.properties` file:
+
+[source]
+----
+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.forbid=${env:UNOMI_FORBID_SCRIPTING_CLASSES:-}
+----
+
+If you encounter any errors while trying to access a class in a condition or
an action it might be due to this
+restrictive configuration.
+
+If you need, for example when adding a custom item type, to adjust these,
please be careful as scripts may be called
+directly from the context.json personalization conditions and therefore should
be kept minimal.
+
==== Automatic profile merging
Apache Unomi is capable of merging profiles based on a common property value.
In order to use this, you must
diff --git a/package/src/main/resources/etc/custom.system.properties
b/package/src/main/resources/etc/custom.system.properties
index f1ad51e..1342313 100644
--- a/package/src/main/resources/etc/custom.system.properties
+++ b/package/src/main/resources/etc/custom.system.properties
@@ -31,7 +31,8 @@
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.forbid=${env:UNOMI_FORBID_SCRIPTING_CLASSES:-}
#######################################################################################################################
## HTTP Settings
##
#######################################################################################################################
diff --git a/package/src/main/resources/etc/org.ops4j.pax.url.mvn.cfg
b/package/src/main/resources/etc/org.ops4j.pax.url.mvn.cfg
new file mode 100644
index 0000000..801642c
--- /dev/null
+++ b/package/src/main/resources/etc/org.ops4j.pax.url.mvn.cfg
@@ -0,0 +1,146 @@
+################################################################################
+#
+# 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.
+#
+################################################################################
+
+#
+# see: https://ops4j1.jira.com/wiki/display/paxurl/Aether+Configuration
+#
+
+# If set to true, the following property will not allow any certificate to be
used
+# when accessing Maven repositories through SSL
+#
+org.ops4j.pax.url.mvn.certificateCheck=true
+
+#
+# Path to the local Maven settings file.
+# The repositories defined in this file will be automatically added to the list
+# of default repositories if the 'org.ops4j.pax.url.mvn.repositories' property
+# below is not set.
+# The following locations are checked for the existence of the settings.xml
file
+# * 1. looks for the specified url
+# * 2. if not found looks for ${user.home}/.m2/settings.xml
+# * 3. if not found looks for ${maven.home}/conf/settings.xml
+# * 4. if not found looks for ${M2_HOME}/conf/settings.xml
+#
+# Properties prefixed with "org.ops4j.pax.url.mvn." have
+# higher priority except <proxies> element. HTTP proxies should be configured
in
+# settings file
+#org.ops4j.pax.url.mvn.settings=
+
+#
+# Path to the local Maven repository which is used to avoid downloading
+# artifacts when they already exist locally.
+# The value of this property will be extracted from the settings.xml file
+# above, or defaulted to:
+# System.getProperty( "user.home" ) + "/.m2/repository"
+# leaving this option commented makes the system dependent on external
+# configuration, which is not always desired
+# "localRepository" is the target location for artifacts downloaded from
+# "remote repositories"
+#org.ops4j.pax.url.mvn.localRepository=
+
+#
+# Default this to false. It's just weird to use undocumented repos
+# "false" means that http://repo1.maven.org/maven2@id=central won't be
+# implicitly used as remote repository
+#
+org.ops4j.pax.url.mvn.useFallbackRepositories=false
+
+#
+# Comma separated list of repositories scanned when resolving an artifact.
+# list of repositories searched in the first place, should contain
+# ${runtime.home}/${karaf.default.repository}.
+# if "org.ops4j.pax.url.mvn.localRepository" is defined and it's not
+# ~/.m2/repository, it's recommended (at least for dev purposes) to add
+# ~/.m2/repository to defaultRepositories
+# each of these repositories is checked by aether as "local repository". if
+# artifact isn't found, "repositories" are searched next
+#
+# Those repositories will be checked before iterating through the
+# below list of repositories and even before the local repository
+# A repository url can be appended with zero or more of the following flags:
+# @snapshots : the repository contains snaphots
+# @noreleases : the repository does not contain any released artifacts
+#
+# The following property value will add the system folder as a repo.
+#
+org.ops4j.pax.url.mvn.defaultRepositories=\
+
${karaf.home.uri}${karaf.default.repository}@id=system.repository@snapshots, \
+ ${karaf.data.uri}kar@id=kar.repository@multi@snapshots, \
+
${karaf.base.uri}${karaf.default.repository}@id=child.system.repository@snapshots
+
+#
+# if "defaultLocalRepoAsRemote" is set do *any* value, localRepository will be
+# added to the list of remote repositories being searched for artifacts
+#
+#org.ops4j.pax.url.mvn.defaultLocalRepoAsRemote = true
+
+#
+# Comma separated list of repositories scanned when resolving an artifact.
+# list of repositories searched after resolution fails for
"defaultRepositories"
+# These are true remote repositories accessed using maven/aether/wagon
+# mechanisms. If any repository contains required artifact, it is then written
+# to "localRepository"
+#
+# if this list is _prepended_ with '+' sign, all repositories from active
+# profiles defined in effective settings.xml file will be _appended_ to this
+# list
+# The default list includes the following repositories:
+# http://repo1.maven.org/maven2@id=central
+#
http://repository.apache.org/content/groups/snapshots-group@id=apache@snapshots@noreleases
+#
https://oss.sonatype.org/content/repositories/snapshots@id=sonatype.snapshots.deploy@snapshots@noreleases
+#
https://oss.sonatype.org/content/repositories/ops4j-snapshots@id=ops4j.sonatype.snapshots.deploy@snapshots@noreleases
+# A repository url can be appended with zero or more of the following flags:
+# @snapshots : the repository contains snapshots
+# @noreleases : the repository does not contain any released artifacts
+# @id=repository.id : the id for the repository, just like in the
+# settings.xml this is optional but recommended
+#
+org.ops4j.pax.url.mvn.repositories= \
+ https://repo1.maven.org/maven2@id=central, \
+
https://repository.apache.org/content/groups/snapshots-group@id=apache@snapshots@noreleases,
\
+
https://oss.sonatype.org/content/repositories/ops4j-snapshots@id=ops4j.sonatype.snapshots.deploy@snapshots@noreleases
+
+#
+# Global policies override repository-specific settings (@checksum=...,
@update=..., @releasesUpdate=..., ...)
+#
+#org.ops4j.pax.url.mvn.globalUpdatePolicy = daily
+#org.ops4j.pax.url.mvn.globalChecksumPolicy = warn
+
+#
+# socket and connection configuration (pax-url-aether 2.5.0)
+#
+# default value for connection and read timeouts, when socket.readTimeout and
socket.connectionTimeout
+# are not specified
+org.ops4j.pax.url.mvn.timeout = 5000
+# timeout in ms when establishing http connection during artifact resolution
+org.ops4j.pax.url.mvn.socket.connectionTimeout = 5000
+# timeout in ms when reading data after connecting to remote repository
+org.ops4j.pax.url.mvn.socket.readTimeout = 30000
+# SO_KEEPALIVE option for sockets, defaults to false
+org.ops4j.pax.url.mvn.socket.keepAlive = false
+# SO_LINGER option for sockets, defaults to -1
+org.ops4j.pax.url.mvn.socket.linger = -1
+# SO_REUSEADDR option for sockets, defaults to false
+org.ops4j.pax.url.mvn.socket.reuseAddress = false
+# TCP_NODELAY option for sockets, defaults to true
+org.ops4j.pax.url.mvn.socket.tcpNoDelay = true
+# Configure buffer size for HTTP connections (output and input buffers),
defaults to 8192 bytes
+org.ops4j.pax.url.mvn.connection.bufferSize = 8192
+# Number of connection retries after failure is detected in http client.
httpclient uses default value "3"
+org.ops4j.pax.url.mvn.connection.retryCount = 3
diff --git a/persistence-elasticsearch/core/pom.xml
b/persistence-elasticsearch/core/pom.xml
index 1745547..b62a426 100644
--- a/persistence-elasticsearch/core/pom.xml
+++ b/persistence-elasticsearch/core/pom.xml
@@ -50,6 +50,12 @@
</dependency>
<dependency>
<groupId>org.apache.unomi</groupId>
+ <artifactId>unomi-common</artifactId>
+ <version>${project.version}</version>
+ <scope>provided</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.apache.unomi</groupId>
<artifactId>unomi-persistence-spi</artifactId>
<version>${project.version}</version>
<scope>provided</scope>
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 d6b18b4..8355015 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,6 +24,7 @@ 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;
@@ -34,10 +35,7 @@ import java.io.IOException;
import java.io.Reader;
import java.io.Serializable;
import java.io.StringReader;
-import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.*;
import java.util.concurrent.ConcurrentHashMap;
public class ConditionContextHelper {
@@ -80,12 +78,7 @@ public class ConditionContextHelper {
return context.get(StringUtils.substringAfter(s,
"parameter::"));
} else if (s.startsWith("script::")) {
String script = StringUtils.substringAfter(s, "script::");
- if (!mvelExpressions.containsKey(script)) {
- ParserConfiguration parserConfiguration = new
ParserConfiguration();
-
parserConfiguration.setClassLoader(ConditionContextHelper.class.getClassLoader());
-
mvelExpressions.put(script,MVEL.compileExpression(script, new
ParserContext(parserConfiguration)));
- }
- return MVEL.executeExpression(mvelExpressions.get(script),
context);
+ return executeScript(context, script);
}
}
} else if (value instanceof Map) {
@@ -111,6 +104,22 @@ 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/plugins/baseplugin/pom.xml b/plugins/baseplugin/pom.xml
index d6279bb..f35fe97 100644
--- a/plugins/baseplugin/pom.xml
+++ b/plugins/baseplugin/pom.xml
@@ -78,6 +78,12 @@
<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>
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 239c321..3b8da80 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
@@ -17,15 +17,13 @@
package org.apache.unomi.plugins.baseplugin.conditions;
-import ognl.Node;
-import ognl.Ognl;
-import ognl.OgnlContext;
-import ognl.OgnlException;
+import ognl.*;
import ognl.enhance.ExpressionAccessor;
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.persistence.elasticsearch.conditions.ConditionContextHelper;
import
org.apache.unomi.persistence.elasticsearch.conditions.ConditionEvaluator;
import
org.apache.unomi.persistence.elasticsearch.conditions.ConditionEvaluatorDispatcher;
@@ -36,6 +34,8 @@ import org.elasticsearch.index.mapper.DateFieldMapper;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import java.lang.reflect.Member;
+import java.lang.reflect.Modifier;
import java.text.SimpleDateFormat;
import java.util.*;
import java.util.function.LongSupplier;
@@ -327,8 +327,10 @@ public class PropertyConditionEvaluator implements
ConditionEvaluator {
}
protected Object getOGNLPropertyValue(Item item, String expression) throws
Exception {
- ExpressionAccessor accessor = getPropertyAccessor(item, expression);
- return accessor != null ? accessor.get((OgnlContext)
Ognl.createDefaultContext(null), item) : null;
+ ClassLoader secureFilteringClassLoader = new
SecureFilteringClassLoader(PropertyConditionEvaluator.class.getClassLoader());
+ OgnlContext ognlContext = getOgnlContext(secureFilteringClassLoader);
+ ExpressionAccessor accessor = getPropertyAccessor(item, expression,
ognlContext, secureFilteringClassLoader);
+ return accessor != null ? accessor.get(ognlContext, item) : null;
}
private Object getNestedPropertyValue(String expressionPart, Map<String,
Object> properties) {
@@ -339,14 +341,48 @@ public class PropertyConditionEvaluator implements
ConditionEvaluator {
if (mapValue == null) {
return null;
}
- String nextExpression = expressionPart.substring(nextDotPos+1);
- return getNestedPropertyValue(nextExpression, (Map<String,Object>)
mapValue);
+ String nextExpression = expressionPart.substring(nextDotPos + 1);
+ return getNestedPropertyValue(nextExpression, (Map<String,
Object>) mapValue);
} else {
return properties.get(expressionPart);
}
}
- private ExpressionAccessor getPropertyAccessor(Item item, String
expression) throws Exception {
+ private class ClassLoaderClassResolver extends DefaultClassResolver {
+ private ClassLoader classLoader;
+
+ public ClassLoaderClassResolver(ClassLoader classLoader) {
+ this.classLoader = classLoader;
+ }
+
+ @Override
+ protected Class toClassForName(String className) throws
ClassNotFoundException {
+ return Class.forName(className, true, classLoader);
+ }
+ }
+
+ private OgnlContext getOgnlContext(ClassLoader classLoader) {
+ return (OgnlContext) Ognl.createDefaultContext(null, new
MemberAccess() {
+ @Override
+ public Object setup(Map context, Object target, Member
member, String propertyName) {
+ return null;
+ }
+
+ @Override
+ public void restore(Map context, Object target, Member
member, String propertyName, Object state) {
+ }
+
+ @Override
+ public boolean isAccessible(Map context, Object target,
Member member, String propertyName) {
+ int modifiers = member.getModifiers();
+ boolean result = Modifier.isPublic(modifiers);
+ return result;
+ }
+ }, new ClassLoaderClassResolver(classLoader),
+ null);
+ }
+
+ private ExpressionAccessor getPropertyAccessor(Item item, String
expression, OgnlContext ognlContext, ClassLoader classLoader) throws Exception {
ExpressionAccessor accessor = null;
String clazz = item.getClass().getName();
Map<String, ExpressionAccessor> expressions =
expressionCache.get(clazz);
@@ -361,8 +397,8 @@ public class PropertyConditionEvaluator implements
ConditionEvaluator {
Thread current = Thread.currentThread();
ClassLoader contextCL = current.getContextClassLoader();
try {
-
current.setContextClassLoader(PropertyConditionEvaluator.class.getClassLoader());
- Node node = Ognl.compileExpression((OgnlContext)
Ognl.createDefaultContext(null), item, expression);
+ current.setContextClassLoader(classLoader);
+ Node node = Ognl.compileExpression(ognlContext, item,
expression);
accessor = node.getAccessor();
} finally {
current.setContextClassLoader(contextCL);
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 fefab83..dff90fb 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
@@ -16,12 +16,14 @@
*/
package org.apache.unomi.plugins.baseplugin.conditions;
+import ognl.MethodFailedException;
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.junit.Test;
+import java.io.File;
import java.util.*;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutorService;
@@ -31,6 +33,7 @@ import java.util.concurrent.Future;
import static junit.framework.TestCase.assertEquals;
import static junit.framework.TestCase.assertNull;
import static
org.apache.unomi.plugins.baseplugin.conditions.PropertyConditionEvaluator.NOT_OPTIMIZED_MARKER;
+import static org.junit.Assert.assertFalse;
public class PropertyConditionEvaluatorTest {
@@ -52,14 +55,14 @@ public class PropertyConditionEvaluatorTest {
assertEquals("Target itemId value is not correct", MOCK_ITEM_ID,
propertyConditionEvaluator.getHardcodedPropertyValue(mockEvent,
"target.itemId"));
assertEquals("Target scope is not correct", DIGITALL_SCOPE,
propertyConditionEvaluator.getHardcodedPropertyValue(mockEvent,
"target.scope"));
assertEquals("Target page path value is not correct", PAGE_PATH_VALUE,
propertyConditionEvaluator.getHardcodedPropertyValue(mockEvent,
"target.properties.pageInfo.pagePath"));
- assertEquals("Target page url value is not correct", PAGE_URL_VALUE,
propertyConditionEvaluator.getHardcodedPropertyValue(mockEvent,
"target.properties.pageInfo.pageURL"));
+ assertEquals("Target page url value is not correct", PAGE_URL_VALUE,
propertyConditionEvaluator.getHardcodedPropertyValue(mockEvent,
"target.properties.pageInfo.pageURL"));
assertEquals("Session size should be 10", 10,
propertyConditionEvaluator.getHardcodedPropertyValue(mockSession, "size"));
assertNull("Unexisting property should be null",
propertyConditionEvaluator.getHardcodedPropertyValue(mockSession,
"systemProperties.goals._csk6r4cgeStartReached"));
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,
"lastEventDate"));
}
@@ -67,11 +70,11 @@ public class PropertyConditionEvaluatorTest {
public void testOGNLEvaluator() throws Exception {
Event mockEvent = generateMockEvent();
assertEquals("Target itemId value is not correct", MOCK_ITEM_ID,
propertyConditionEvaluator.getOGNLPropertyValue(mockEvent, "target.itemId"));
- assertEquals("Target scope is not correct",
DIGITALL_SCOPE,propertyConditionEvaluator.getOGNLPropertyValue(mockEvent,
"target.scope"));
+ assertEquals("Target scope is not correct", DIGITALL_SCOPE,
propertyConditionEvaluator.getOGNLPropertyValue(mockEvent, "target.scope"));
assertEquals("Target page path value is not correct", PAGE_PATH_VALUE,
propertyConditionEvaluator.getOGNLPropertyValue(mockEvent,
"target.properties.pageInfo.pagePath"));
assertEquals("Target page url value is not correct", PAGE_URL_VALUE,
propertyConditionEvaluator.getOGNLPropertyValue(mockEvent,
"target.properties.pageInfo.pageURL"));
assertEquals("Session size should be 10", 10,
propertyConditionEvaluator.getOGNLPropertyValue(mockSession, "size"));
- assertEquals("Should have received the proper last even date",
SESSION_LAST_EVENT_DATE,
propertyConditionEvaluator.getOGNLPropertyValue(mockSession, "lastEventDate") );
+ assertEquals("Should have received the proper last even date",
SESSION_LAST_EVENT_DATE,
propertyConditionEvaluator.getOGNLPropertyValue(mockSession, "lastEventDate"));
assertNull("Unexisting property should be null",
propertyConditionEvaluator.getHardcodedPropertyValue(mockSession,
"systemProperties.goals._csk6r4cgeStartReached"));
assertNull("Unexisting property should be null",
propertyConditionEvaluator.getHardcodedPropertyValue(mockProfile,
"properties.email"));
@@ -97,7 +100,7 @@ public class PropertyConditionEvaluatorTest {
public void testPropertyEvaluator() throws Exception {
Event mockEvent = generateMockEvent();
assertEquals("Target itemId value is not correct", MOCK_ITEM_ID,
propertyConditionEvaluator.getPropertyValue(mockEvent, "target.itemId"));
- assertEquals("Target scope is not correct",
DIGITALL_SCOPE,propertyConditionEvaluator.getPropertyValue(mockEvent,
"target.scope"));
+ assertEquals("Target scope is not correct", DIGITALL_SCOPE,
propertyConditionEvaluator.getPropertyValue(mockEvent, "target.scope"));
assertEquals("Target page path value is not correct", PAGE_PATH_VALUE,
propertyConditionEvaluator.getPropertyValue(mockEvent,
"target.properties.pageInfo.pagePath"));
assertNull("Unexisting property should be null",
propertyConditionEvaluator.getPropertyValue(mockSession,
"systemProperties.goals._csk6r4cgeStartReached"));
@@ -107,6 +110,35 @@ public class PropertyConditionEvaluatorTest {
assertEquals("Session last event date is not right",
SESSION_LAST_EVENT_DATE,
propertyConditionEvaluator.getPropertyValue(mockSession, "lastEventDate"));
}
+ @Test
+ public void testOGNLSecurity() throws Exception {
+ Event mockEvent = generateMockEvent();
+ File vulnFile = new File("target/vuln-file.txt");
+ if (vulnFile.exists()) {
+ vulnFile.delete();
+ }
+ try {
+ propertyConditionEvaluator.getOGNLPropertyValue(mockEvent,
"@java.lang.Runtime@getRuntime().exec('touch " + vulnFile.getCanonicalPath() +
"')");
+ } catch (RuntimeException | MethodFailedException re) {
+ // we ignore these exceptions as they are expected.
+ }
+ assertFalse("Vulnerability successfully executed ! File created at " +
vulnFile.getCanonicalPath(), vulnFile.exists());
+ try {
+ propertyConditionEvaluator.getOGNLPropertyValue(mockEvent,
"(#cmd='touch " + vulnFile.getCanonicalPath() +
"').(#cmds={'bash','-c',#cmd}).(#p=new
java.lang.ProcessBuilder(#cmds)).(#p.redirectErrorStream(true)).(#process=#p.start())");
+ } 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 " +
vulnFile.getCanonicalPath(), vulnFile.exists());
+ try {
+ propertyConditionEvaluator.getOGNLPropertyValue(mockEvent,
"(#cmd='touch " + vulnFile.getCanonicalPath() +
"').(#iswin=(@java.lang.System@getProperty('os.name').toLowerCase().contains('win'))).(#cmds=(#iswin?{'cmd.exe','/c',#cmd}:{'bash','-c',#cmd})).(#p=new
java.lang.ProcessBuilder(#cmds)).(#p.redirectErrorStream(true)).(#process=#p.start())");
+ } 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 " +
vulnFile.getCanonicalPath(), vulnFile.exists());
+ }
+
private void runHardcodedTest(int workerCount, ExecutorService
executorService) throws InterruptedException {
List<Callable<Object>> todo = new
ArrayList<Callable<Object>>(workerCount);
long startTime = System.currentTimeMillis();
@@ -135,7 +167,7 @@ public class PropertyConditionEvaluatorTest {
targetItem.setItemId(MOCK_ITEM_ID);
targetItem.setScope(DIGITALL_SCOPE);
mockEvent.setTarget(targetItem);
- Map<String,Object> pageInfoMap = new HashMap<>();
+ Map<String, Object> pageInfoMap = new HashMap<>();
pageInfoMap.put("pagePath", PAGE_PATH_VALUE);
pageInfoMap.put("pageURL", PAGE_URL_VALUE);
targetItem.getProperties().put("pageInfo", pageInfoMap);
diff --git a/pom.xml b/pom.xml
index db8493f..a67de95 100644
--- a/pom.xml
+++ b/pom.xml
@@ -891,7 +891,7 @@
<dependency>
<groupId>ognl</groupId>
<artifactId>ognl</artifactId>
- <version>3.0.11</version>
+ <version>3.2.14</version>
</dependency>
<dependency>
<groupId>javassist</groupId>
diff --git a/services/pom.xml b/services/pom.xml
index f081a98..87b46d2 100644
--- a/services/pom.xml
+++ b/services/pom.xml
@@ -134,6 +134,13 @@
</dependency>
<dependency>
+ <groupId>org.apache.unomi</groupId>
+ <artifactId>unomi-common</artifactId>
+ <version>${project.version}</version>
+ <scope>provided</scope>
+ </dependency>
+
+ <dependency>
<groupId>com.github.seancfoley</groupId>
<artifactId>ipaddress</artifactId>
<version>4.3.0</version>
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 6306be6..0a0d951 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
@@ -23,6 +23,7 @@ import org.apache.unomi.api.Event;
import org.apache.unomi.api.actions.Action;
import org.apache.unomi.api.actions.ActionExecutor;
import org.apache.unomi.api.services.EventService;
+import org.apache.unomi.common.SecureFilteringClassLoader;
import org.apache.unomi.metrics.MetricAdapter;
import org.apache.unomi.metrics.MetricsService;
import org.mvel2.MVEL;
@@ -89,23 +90,9 @@ public class ActionExecutorDispatcher {
valueExtractors.put("script", new ValueExtractor() {
@Override
public Object extract(String valueAsString, Event event) throws
IllegalAccessException, NoSuchMethodException, InvocationTargetException {
- final ClassLoader tccl =
Thread.currentThread().getContextClassLoader();
- try {
-
Thread.currentThread().setContextClassLoader(getClass().getClassLoader());
- if (!mvelExpressions.containsKey(valueAsString)) {
- ParserConfiguration parserConfiguration = new
ParserConfiguration();
-
parserConfiguration.setClassLoader(getClass().getClassLoader());
- 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);
- }
+ return executeScript(valueAsString, event);
}
+
});
}
@@ -202,4 +189,24 @@ public class ActionExecutorDispatcher {
Object extract(String valueAsString, Event event) throws
IllegalAccessException, NoSuchMethodException, InvocationTargetException;
}
+ 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);
+ }
+ }
+
}
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
new file mode 100644
index 0000000..813c067
--- /dev/null
+++
b/services/src/test/java/org/apache/unomi/services/actions/ActionExecutorDispatcherTest.java
@@ -0,0 +1,106 @@
+/*
+ * 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;
+ }
+
+}
