MNT-21009 : Sandboxed arbitrary Code Execution (#657)

* MNT-21009 :  Sandboxed  arbitrary Code Execution

* MNT-21009: added basic code java for javascrrict security check.

* MNT-21009 : added release for MNT-21009

* [maven-release-plugin][skip ci] prepare release alfresco-repository-MNT-21009

* [maven-release-plugin][skip ci] prepare for next development iteration

* MNT-21009: added sandboxNativeObject in case of calling getClass using reflection.

* [maven-release-plugin][skip ci] prepare release alfresco-repository-MNT-21009

* MNT-21009 : changes after review.

.travis.yml

@@ -18,6 +18,7 @@ branches:
   only:
     - master
     - /support\/.*/
+    - fix/MNT-21009-arbitrary-codeExecution
 
 stages:
   - test
@@ -110,7 +111,7 @@ jobs:
       script: travis_wait 20 mvn test -B -Dtest=AllDBTestsTestSuite -Ddb.name=alfresco -Ddb.url=jdbc:mariadb://localhost:3307/alfresco?useUnicode=yes\&characterEncoding=UTF-8 -Ddb.username=alfresco -Ddb.password=alfresco -Ddb.driver=org.mariadb.jdbc.Driver
     - stage: release
       name: "Push to Nexus"
-      if: fork = false AND (branch = master OR branch =~ /support\/.*/) AND type != pull_request AND commit_message !~ /\[no-release\]/
+      if: fork = false AND (branch = master OR branch =~ /support\/.*/ OR branch = fix/MNT-21009-arbitrary-codeExecution) AND type != pull_request AND commit_message !~ /\[no-release\]/
       before_install:
         - "cp .travis.settings.xml $HOME/.m2/settings.xml"
       script:
@@ -119,4 +120,4 @@ jobs:
         # Add email to link commits to user
         - git config user.email "${GIT_EMAIL}"
         # Skip building of release commits
-        - mvn --batch-mode -q -DscmCommentPrefix="[maven-release-plugin][skip ci] " -Dusername="${GIT_USERNAME}" -Dpassword="${GIT_PASSWORD}" -DskipTests -Darguments=-DskipTests release:clean release:prepare release:perform
+        - mvn --batch-mode -q -DscmCommentPrefix="[maven-release-plugin][skip ci] " -Dusername="${GIT_USERNAME}" -Dpassword="${GIT_PASSWORD}" -DskipTests -Darguments=-DskipTests -DreleaseVersion=MNT-21009 -DdevelopmentVersion=8.22-SNAPSHOT release:clean release:prepare release:perform

pom.xml

@@ -4,7 +4,6 @@
     <name>Alfresco Repository</name>
     <version>8.24-SNAPSHOT</version>
     <packaging>jar</packaging>
-
     <parent>
         <groupId>org.alfresco</groupId>
         <artifactId>alfresco-super-pom</artifactId>

src/main/java/org/alfresco/repo/jscript/RhinoScriptProcessor.java

@@ -79,7 +79,10 @@ public class RhinoScriptProcessor extends BaseProcessor implements ScriptProcess
     private static final String PATH_CLASSPATH = "classpath:";
     
     /** Wrap Factory */
-    private static final WrapFactory wrapFactory = new RhinoWrapFactory();
+    private static final WrapFactory wrapFactory = new RhinoWrapFactory();
+    
+    /** Sandbox Wrap Factory */
+    private static final SandboxWrapFactory sandboxFactory = new SandboxWrapFactory();
     
     /** Base Value Converter */
     private final ValueConverter valueConverter = new ValueConverter();
@@ -453,14 +456,15 @@ public class RhinoScriptProcessor extends BaseProcessor implements ScriptProcess
         {
             // Create a thread-specific scope from one of the shared scopes.
             // See http://www.mozilla.org/rhino/scopes.html
-            cx.setWrapFactory(wrapFactory);
+            cx.setWrapFactory(secure ? wrapFactory : sandboxFactory);
             Scriptable scope;
             if (this.shareSealedScopes)
             {
                 Scriptable sharedScope = secure ? this.nonSecureScope : this.secureScope;
                 scope = cx.newObject(sharedScope);
                 scope.setPrototype(sharedScope);
-                scope.setParentScope(null);
+                scope.setParentScope(null);
+
             }
             else
             {
@@ -578,23 +582,44 @@ public class RhinoScriptProcessor extends BaseProcessor implements ScriptProcess
 
     
     /**
-     * Rhino script value wraper
+     * Rhino script value wrapper
      */
     private static class RhinoWrapFactory extends WrapFactory
-    {
+    {
+        protected Scriptable wrapBasicJavaObject(Context cx, Scriptable scope, Object javaObject, Class<?> staticType)
+        {
+            return super.wrapAsJavaObject(cx, scope, javaObject, staticType);
+        }
+        
     	/* (non-Javadoc)
     	 * @see org.mozilla.javascript.WrapFactory#wrapAsJavaObject(org.mozilla.javascript.Context, org.mozilla.javascript.Scriptable, java.lang.Object, java.lang.Class)
     	 */
-        public Scriptable wrapAsJavaObject(Context cx, Scriptable scope, Object javaObject, Class staticType)
+        public Scriptable wrapAsJavaObject(Context cx, Scriptable scope, Object javaObject, Class<?> staticType)
-        {
+        {
-            if (javaObject instanceof Map && !(javaObject instanceof ScriptableHashMap))
+            if (javaObject instanceof Map && !(javaObject instanceof ScriptableHashMap))
-            {
+            {
-                return new NativeMap(scope, (Map)javaObject);
+                return new NativeMap(scope, (Map) javaObject);
-            }
+            }
-            return super.wrapAsJavaObject(cx, scope, javaObject, staticType);
+            return wrapBasicJavaObject(cx, scope, javaObject, staticType);
-        }
+        }
+    }
+    
+    /**
+     * A {@link WrapFactory} that ensures {@link org.mozilla.javascript.NativeJavaObject} instances are of the
+     * {@link SandboxNativeJavaObject} variety.
+     */
+    private static class SandboxWrapFactory extends RhinoWrapFactory
+    {
+        /* (non-Javadoc)
+         * @see org.mozilla.javascript.WrapFactory#wrapAsJavaObject(org.mozilla.javascript.Context, org.mozilla.javascript.Scriptable, java.lang.Object, java.lang.Class)
+         */
+        @Override
+        protected Scriptable wrapBasicJavaObject(Context cx, Scriptable scope, Object javaObject, Class<?> staticType)
+        {
+            return new SandboxNativeJavaObject(scope, javaObject, staticType);
+        }
+
     }
-
     
     /**
      * Pre initializes two scope objects (one secure and one not) with the standard objects preinitialised.
@@ -621,7 +646,7 @@ public class RhinoScriptProcessor extends BaseProcessor implements ScriptProcess
         cx = Context.enter();
         try
         {
-            cx.setWrapFactory(wrapFactory);
+            cx.setWrapFactory(sandboxFactory);
             this.nonSecureScope = initScope(cx, true, true);
         }
         finally
@@ -643,27 +668,23 @@ public class RhinoScriptProcessor extends BaseProcessor implements ScriptProcess
      *                  is to be reused.
      * @return the scope object
      */
-    protected Scriptable initScope(Context cx, boolean secure, boolean sealed)
+    protected Scriptable initScope(Context cx, boolean secure, boolean sealed)
-    {
+    {
-        Scriptable scope;
+        Scriptable scope;
-        if (secure)
+        if (secure)
-        {
+        {
-            // Initialise the non-secure scope
+            // Initialise the non-secure scope
-            // allow access to all libraries and objects, including the importer
+            // allow access to all libraries and objects, including the importer
-            // @see http://www.mozilla.org/rhino/ScriptingJava.html
+            // @see http://www.mozilla.org/rhino/ScriptingJava.html
-            scope = new ImporterTopLevel(cx, sealed);
+            scope = new ImporterTopLevel(cx, sealed);
-        }
+        }
-        else
+        else
-        {
+        {
-            // Initialise the secure scope
+            // Initialise the secure scope
-            scope = cx.initStandardObjects(null, sealed);
+            // This sets up "scope" to have access to all the standard JavaScript classes, but does not create global objects for any top-level Java packages.
-            // remove security issue related objects - this ensures the script may not access
+            // In addition, the "Packages," "JavaAdapter," and "JavaImporter" classes, and the "getClass" function, are not initialized.
-            // unsecure java.* libraries or import any other classes for direct access - only
+            scope = cx.initSafeStandardObjects(null, sealed);
-            // the configured root host objects will be available to the script writer
+        }
-            scope.delete("Packages");
+        return scope;
-            scope.delete("getClass");
-            scope.delete("java");
-        }
-        return scope;
     }
 }

src/main/java/org/alfresco/repo/jscript/SandboxNativeJavaObject.java

@@ -0,0 +1,52 @@
+/*
+ * #%L
+ * Alfresco Repository
+ * %%
+ * Copyright (C) 2005 - 2016 Alfresco Software Limited
+ * %%
+ * This file is part of the Alfresco software. 
+ * If the software was purchased under a paid Alfresco license, the terms of 
+ * the paid license agreement will prevail.  Otherwise, the software is 
+ * provided under the following open source license terms:
+ * 
+ * Alfresco is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ * 
+ * Alfresco is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ * 
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
+ * #L%
+ */
+package org.alfresco.repo.jscript;
+
+import org.mozilla.javascript.NativeJavaObject;
+import org.mozilla.javascript.Scriptable;
+
+
+/**
+ * A sandboxed {@link NativeJavaObject} that prevents using reflection to escape a sandbox.
+ */
+public class SandboxNativeJavaObject extends NativeJavaObject
+{
+    public SandboxNativeJavaObject(Scriptable scope, Object javaObject, Class<?> staticType)
+    {
+        super(scope, javaObject, staticType);
+    }
+
+    @Override
+    public Object get(String name, Scriptable start)
+    {
+        if ("getClass".equals(name))
+        {
+            return NOT_FOUND;
+        }
+        return super.get(name, start);
+    }
+
+}

src/test/java/org/alfresco/repo/jscript/RhinoScriptTest.java

@@ -25,19 +25,24 @@
  */
 package org.alfresco.repo.jscript;
 
+import static org.junit.Assert.fail;
+
+import java.io.IOException;
 import java.io.InputStream;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
-import junit.framework.TestCase;
+import junit.framework.TestCase;
-
+
+import org.alfresco.error.AlfrescoRuntimeException;
 import org.alfresco.model.ContentModel;
 import org.alfresco.repo.dictionary.DictionaryComponent;
 import org.alfresco.repo.dictionary.DictionaryDAO;
 import org.alfresco.repo.dictionary.M2Model;
-import org.alfresco.repo.node.BaseNodeServiceTest;
+import org.alfresco.repo.node.BaseNodeServiceTest;
-import org.alfresco.repo.security.authentication.AuthenticationComponent;
+import org.alfresco.repo.security.authentication.AuthenticationComponent;
+import org.alfresco.repo.security.permissions.AccessDeniedException;
 import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback;
 import org.alfresco.service.ServiceRegistry;
 import org.alfresco.service.cmr.repository.ChildAssociationRef;
@@ -49,13 +54,14 @@ import org.alfresco.service.cmr.repository.ScriptService;
 import org.alfresco.service.cmr.repository.StoreRef;
 import org.alfresco.service.namespace.QName;
 import org.alfresco.service.transaction.TransactionService;
-import org.alfresco.test_category.OwnJVMTestsCategory;
+import org.alfresco.test_category.OwnJVMTestsCategory;
-import org.alfresco.util.ApplicationContextHelper;
+import org.alfresco.util.ApplicationContextHelper;
 import org.junit.experimental.categories.Category;
 import org.mozilla.javascript.Context;
 import org.mozilla.javascript.Scriptable;
 import org.mozilla.javascript.ScriptableObject;
-import org.springframework.context.ApplicationContext;
+import org.springframework.context.ApplicationContext;
+
 
 /**
  * @author Kevin Roast
@@ -358,6 +364,75 @@ public class RhinoScriptTest extends TestCase
                     return null;
                 }                
             });
+    }
+    
+    // MNT-21009
+    public void testUnsecureScriptAddedOnRepoNode()
+    {
+
+        transactionService.getRetryingTransactionHelper().doInTransaction(new RetryingTransactionCallback<Object>()
+        {
+            public Object execute() throws Exception
+            {
+                StoreRef store = nodeService.createStore(StoreRef.PROTOCOL_WORKSPACE, "rhino_" + System.currentTimeMillis());
+                NodeRef root = nodeService.getRootNode(store);
+                BaseNodeServiceTest.buildNodeGraph(nodeService, root);
+
+                try
+                {
+                    Map<String, Object> model = new HashMap<String, Object>();
+                    model.put("out", System.out);
+
+                    // create an Alfresco scriptable Node object
+                    // the Node object is a wrapper similar to the TemplateNode
+                    // concept
+                    ScriptNode rootNode = new ScriptNode(root, serviceRegistry, null);
+                    model.put("root", rootNode);
+
+                    // test executing a script embedded inside Node content
+                    ChildAssociationRef childRef = nodeService.createNode(root, BaseNodeServiceTest.ASSOC_TYPE_QNAME_TEST_CHILDREN,
+                            QName.createQName(BaseNodeServiceTest.NAMESPACE, "script_content"), BaseNodeServiceTest.TYPE_QNAME_TEST_CONTENT, null);
+                    NodeRef contentNodeRef = childRef.getChildRef();
+                    ContentWriter writer = contentService.getWriter(contentNodeRef, BaseNodeServiceTest.PROP_QNAME_TEST_CONTENT, true);
+                    writer.setMimetype("application/x-javascript");
+                    writer.putContent(BASIC_JAVA);
+
+                    try
+                    {
+                        scriptService.executeScript(contentNodeRef, BaseNodeServiceTest.PROP_QNAME_TEST_CONTENT, model);
+                        fail("execution of nonsecure script on nodeRef is not allowed.");
+                    }
+                    catch (AlfrescoRuntimeException ex)
+                    {
+                        // expected
+                    }
+                    
+                    
+                    ChildAssociationRef childRef1 = nodeService.createNode(root, BaseNodeServiceTest.ASSOC_TYPE_QNAME_TEST_CHILDREN,
+                            QName.createQName(BaseNodeServiceTest.NAMESPACE, "script_content"), BaseNodeServiceTest.TYPE_QNAME_TEST_CONTENT, null);
+                    NodeRef contentNodeRef1 = childRef1.getChildRef();
+                    ContentWriter writer1 = contentService.getWriter(contentNodeRef1, BaseNodeServiceTest.PROP_QNAME_TEST_CONTENT, true);
+                    writer1.setMimetype("application/x-javascript");
+                    writer1.putContent(REFLECTION_GET_CLASS);
+                    
+                    try
+                    {
+                        scriptService.executeScript(contentNodeRef1, BaseNodeServiceTest.PROP_QNAME_TEST_CONTENT, model);
+                        fail("execution of nonsecure script on nodeRef is not allowed.");
+                    }
+                    catch (AlfrescoRuntimeException ex)
+                    {
+                        // expected
+                    }
+                }
+                finally
+                {
+                }
+
+                return null;
+            }
+        });
+
     }
     
     private static final String TESTSCRIPT_CLASSPATH1 = "org/alfresco/repo/jscript/test_script1.js";
@@ -377,5 +452,14 @@ public class RhinoScriptTest extends TestCase
             "var childByNameNode = root.childByNamePath(\"/\" + childList[0].name);\r\n" +
             "logger.log(\"child by name path: \" + childByNameNode.name);\r\n" +
             "var xpathResults = root.childrenByXPath(\"/*\");\r\n" +
-            "logger.log(\"children of root from xpath: \" + xpathResults.length);\r\n";
+            "logger.log(\"children of root from xpath: \" + xpathResults.length);\r\n";
+    
+    private static final String BASIC_JAVA = 
+            "var list = com.google.common.collect.Lists.newArrayList();\n" + 
+            "root.nodeRef.getClass().forName(\"java.lang.ProcessBuilder\")";
+    
+    private static final String REFLECTION_GET_CLASS =
+          "root.nodeRef.getClass().forName(\"java.lang.ProcessBuilder\")";
+            
+
 }