You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by rm...@apache.org on 2019/12/18 12:36:34 UTC

[lucene-solr] branch branch_8x updated: SOLR-14110: sandbox javax.script usage in tests

This is an automated email from the ASF dual-hosted git repository.

rmuir pushed a commit to branch branch_8x
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git


The following commit(s) were added to refs/heads/branch_8x by this push:
     new 53a82ae  SOLR-14110: sandbox javax.script usage in tests
53a82ae is described below

commit 53a82aedcb4c62ac8b2573e7caa6104ce7db8229
Author: Robert Muir <rm...@apache.org>
AuthorDate: Wed Dec 18 06:30:24 2019 -0500

    SOLR-14110: sandbox javax.script usage in tests
---
 .../solr/handler/dataimport/ScriptTransformer.java | 33 ++++++++++++++++++++--
 .../handler/dataimport/TestScriptTransformer.java  | 22 +++++++++++++++
 .../StatelessScriptUpdateProcessorFactory.java     | 33 ++++++++++++++++++++--
 .../src/test-files/solr/collection1/conf/evil.js   | 21 ++++++++++++++
 .../conf/solrconfig-script-updateprocessor.xml     |  6 ++++
 .../StatelessScriptUpdateProcessorFactoryTest.java | 10 +++++++
 6 files changed, 121 insertions(+), 4 deletions(-)

diff --git a/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/ScriptTransformer.java b/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/ScriptTransformer.java
index 165d76d..fe848b1 100644
--- a/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/ScriptTransformer.java
+++ b/solr/contrib/dataimporthandler/src/java/org/apache/solr/handler/dataimport/ScriptTransformer.java
@@ -19,6 +19,12 @@ package org.apache.solr.handler.dataimport;
 import static org.apache.solr.handler.dataimport.DataImportHandlerException.wrapAndThrow;
 import static org.apache.solr.handler.dataimport.DataImportHandlerException.SEVERE;
 
+import java.security.AccessControlContext;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
+import java.security.PrivilegedActionException;
+import java.security.PrivilegedExceptionAction;
+import java.security.ProtectionDomain;
 import java.util.Map;
 
 import javax.script.Invocable;
@@ -46,7 +52,16 @@ public class ScriptTransformer extends Transformer {
  private String functionName;
 
   @Override
-  public Object transformRow(Map<String, Object> row, Context context) {
+  public Object transformRow(Map<String,Object> row, Context context) {
+    return AccessController.doPrivileged(new PrivilegedAction<Object>() {
+      @Override
+      public Object run() {
+        return transformRowUnsafe(row, context);
+      }
+    }, SCRIPT_SANDBOX);
+  }
+
+  public Object transformRowUnsafe(Map<String, Object> row, Context context) {
     try {
       if (engine == null)
         initEngine(context);
@@ -84,7 +99,17 @@ public class ScriptTransformer extends Transformer {
               + scriptEngine.getClass().getName());
     }
     try {
-      scriptEngine.eval(scriptText);
+      try {
+        AccessController.doPrivileged(new PrivilegedExceptionAction<Void>() {
+          @Override
+          public Void run() throws ScriptException  {
+            scriptEngine.eval(scriptText);
+            return null;
+          }
+        }, SCRIPT_SANDBOX);
+      } catch (PrivilegedActionException e) {
+        throw (ScriptException) e.getException();
+      }
     } catch (ScriptException e) {
       wrapAndThrow(SEVERE, e, "'eval' failed with language: " + scriptLang
           + " and script: \n" + scriptText);
@@ -99,4 +124,8 @@ public class ScriptTransformer extends Transformer {
     return functionName;
   }
 
+  // sandbox for script code: zero permissions
+  private static final AccessControlContext SCRIPT_SANDBOX =
+      new AccessControlContext(new ProtectionDomain[] { new ProtectionDomain(null, null) });
+
 }
diff --git a/solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestScriptTransformer.java b/solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestScriptTransformer.java
index c106f8e..9cd606d 100644
--- a/solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestScriptTransformer.java
+++ b/solr/contrib/dataimporthandler/src/test/org/apache/solr/handler/dataimport/TestScriptTransformer.java
@@ -56,6 +56,28 @@ public class TestScriptTransformer extends AbstractDataImportHandlerTestCase {
     }
   }
 
+  @Test
+  public void testEvil() {
+    assumeTrue("This test only works with security manager", System.getSecurityManager() != null);
+    String script = "function f1(row) {"
+            + "var os = Packages.java.lang.System.getProperty('os.name');"
+            + "row.put('name', os);"
+            + "return row;\n"
+            + "}";
+
+    Context context = getContext("f1", script);
+    Map<String, Object> map = new HashMap<>();
+    map.put("name", "Scott");
+    EntityProcessorWrapper sep = new EntityProcessorWrapper(new SqlEntityProcessor(), null, null);
+    sep.init(context);
+    DataImportHandlerException expected = expectThrows(DataImportHandlerException.class, () -> {
+      sep.applyTransformer(map);
+    });
+    assumeFalse("This JVM does not have JavaScript installed.  Test Skipped.",
+        expected.getMessage().startsWith("Cannot load Script Engine for language"));
+    assertTrue(expected.getCause().toString(), SecurityException.class.isAssignableFrom(expected.getCause().getClass()));
+  }
+
   private Context getContext(String funcName, String script) {
     List<Map<String, String>> fields = new ArrayList<>();
     Map<String, String> entity = new HashMap<>();
diff --git a/solr/core/src/java/org/apache/solr/update/processor/StatelessScriptUpdateProcessorFactory.java b/solr/core/src/java/org/apache/solr/update/processor/StatelessScriptUpdateProcessorFactory.java
index c66f183..10f82ad 100644
--- a/solr/core/src/java/org/apache/solr/update/processor/StatelessScriptUpdateProcessorFactory.java
+++ b/solr/core/src/java/org/apache/solr/update/processor/StatelessScriptUpdateProcessorFactory.java
@@ -41,6 +41,12 @@ import java.io.InputStream;
 import java.io.Reader;
 import java.lang.invoke.MethodHandles;
 import java.nio.charset.StandardCharsets;
+import java.security.AccessControlContext;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
+import java.security.PrivilegedActionException;
+import java.security.PrivilegedExceptionAction;
+import java.security.ProtectionDomain;
 import java.util.Set;
 import java.util.LinkedHashSet;
 import java.util.ArrayList;
@@ -269,7 +275,7 @@ public class StatelessScriptUpdateProcessorFactory extends UpdateRequestProcesso
     }
 
     for (ScriptFile scriptFile : scriptFiles) {
-      ScriptEngine engine = null;
+      final ScriptEngine engine;
       if (null != engineName) {
         engine = scriptEngineManager.getEngineByName(engineName);
         if (engine == null) {
@@ -314,7 +320,17 @@ public class StatelessScriptUpdateProcessorFactory extends UpdateRequestProcesso
         Reader scriptSrc = scriptFile.openReader(resourceLoader);
   
         try {
-          engine.eval(scriptSrc);
+          try {
+            AccessController.doPrivileged(new PrivilegedExceptionAction<Void>() {
+              @Override
+              public Void run() throws ScriptException  {
+                engine.eval(scriptSrc);
+                return null;
+              }
+            }, SCRIPT_SANDBOX);
+          } catch (PrivilegedActionException e) {
+            throw (ScriptException) e.getException();
+          }
         } catch (ScriptException e) {
           throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, 
                                   "Unable to evaluate script: " + 
@@ -424,6 +440,15 @@ public class StatelessScriptUpdateProcessorFactory extends UpdateRequestProcesso
      * cast to a java Boolean.
      */
     private boolean invokeFunction(String name, Object... cmd) {
+      return AccessController.doPrivileged(new PrivilegedAction<Boolean>() {
+        @Override
+        public Boolean run() {
+          return invokeFunctionUnsafe(name, cmd);
+        }
+      }, SCRIPT_SANDBOX);
+    }
+
+    private boolean invokeFunctionUnsafe(String name, Object... cmd) {
 
       for (EngineInfo engine : engines) {
         try {
@@ -496,4 +521,8 @@ public class StatelessScriptUpdateProcessorFactory extends UpdateRequestProcesso
         (input, StandardCharsets.UTF_8);
     }
   }
+
+  // sandbox for script code: zero permissions
+  private static final AccessControlContext SCRIPT_SANDBOX =
+      new AccessControlContext(new ProtectionDomain[] { new ProtectionDomain(null, null) });
 }
diff --git a/solr/core/src/test-files/solr/collection1/conf/evil.js b/solr/core/src/test-files/solr/collection1/conf/evil.js
new file mode 100644
index 0000000..d494814
--- /dev/null
+++ b/solr/core/src/test-files/solr/collection1/conf/evil.js
@@ -0,0 +1,21 @@
+var sys = Packages.java.lang.System;
+
+function processAdd(cmd) {
+  sys.getProperty("os.name");
+}
+
+function processDelete(cmd) {
+}
+
+function processMergeIndexes(cmd) {
+}
+
+function processCommit(cmd) {
+}
+
+function processRollback(cmd) {
+}
+
+function finish() {
+}
+
diff --git a/solr/core/src/test-files/solr/collection1/conf/solrconfig-script-updateprocessor.xml b/solr/core/src/test-files/solr/collection1/conf/solrconfig-script-updateprocessor.xml
index f87385e..74f00fd 100644
--- a/solr/core/src/test-files/solr/collection1/conf/solrconfig-script-updateprocessor.xml
+++ b/solr/core/src/test-files/solr/collection1/conf/solrconfig-script-updateprocessor.xml
@@ -117,4 +117,10 @@
     </processor>
   </updateRequestProcessorChain>
 
+  <updateRequestProcessorChain name="evil">
+    <processor class="solr.StatelessScriptUpdateProcessorFactory">
+      <str name="script">evil.js</str>
+    </processor>
+  </updateRequestProcessorChain>
+
 </config>
diff --git a/solr/core/src/test/org/apache/solr/update/processor/StatelessScriptUpdateProcessorFactoryTest.java b/solr/core/src/test/org/apache/solr/update/processor/StatelessScriptUpdateProcessorFactoryTest.java
index 04c277c..09dd783 100644
--- a/solr/core/src/test/org/apache/solr/update/processor/StatelessScriptUpdateProcessorFactoryTest.java
+++ b/solr/core/src/test/org/apache/solr/update/processor/StatelessScriptUpdateProcessorFactoryTest.java
@@ -259,4 +259,14 @@ public class StatelessScriptUpdateProcessorFactoryTest extends UpdateProcessorTe
 
   }
 
+  public void testScriptSandbox() throws Exception  {
+    assumeTrue("This test only works with security manager", System.getSecurityManager() != null);
+    expectThrows(SecurityException.class, () -> {
+      processAdd("evil",
+        doc(f("id", "5"),
+            f("name", " foo "),
+            f("subject", "BAR")));
+    });
+  }
+
 }