[dolphinscheduler] branch dev updated: Script cannot contains ''' in params (#12067)

[caishunfeng]

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

caishunfeng pushed a commit to branch dev
in repository https://gitbox.apache.org/repos/asf/dolphinscheduler.git


The following commit(s) were added to refs/heads/dev by this push:
     new cb4ef54a0e Script cannot contains ''' in params (#12067)
cb4ef54a0e is described below

commit cb4ef54a0e7d3bbfa83c244afcc5d2a518cc34c2
Author: Wenjun Ruan <wen...@apache.org>
AuthorDate: Wed Sep 21 08:52:51 2022 +0800

    Script cannot contains ''' in params (#12067)
---
 .../plugin/alert/script/ScriptSender.java          | 32 ++++++++++++++++++----
 .../plugin/alert/script/ScriptSenderTest.java      |  8 ++++++
 2 files changed, 34 insertions(+), 6 deletions(-)

diff --git 
a/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/main/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptSender.java
 
b/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/main/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptSender.java
index 7f255803c4..3f6e690f03 100644
--- 
a/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/main/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptSender.java
+++ 
b/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/main/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptSender.java
@@ -18,13 +18,15 @@
 package org.apache.dolphinscheduler.plugin.alert.script;
 
 import org.apache.dolphinscheduler.alert.api.AlertResult;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 
 import java.io.File;
 import java.util.Map;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
 public final class ScriptSender {
+
     private static final Logger logger = 
LoggerFactory.getLogger(ScriptSender.class);
     private static final String ALERT_TITLE_OPTION = " -t ";
     private static final String ALERT_CONTENT_OPTION = " -c ";
@@ -54,22 +56,40 @@ public final class ScriptSender {
             alertResult.setMessage("shell script not support windows os");
             return alertResult;
         }
-        //validate script path in case of injections
+        // validate script path in case of injections
         File shellScriptFile = new File(scriptPath);
-        //validate existence
+        // validate existence
         if (!shellScriptFile.exists()) {
             logger.error("shell script not exist : {}", scriptPath);
             alertResult.setMessage("shell script not exist : " + scriptPath);
             return alertResult;
         }
-        //validate is file
+        // validate is file
         if (!shellScriptFile.isFile()) {
             logger.error("shell script is not a file : {}", scriptPath);
             alertResult.setMessage("shell script is not a file : " + 
scriptPath);
             return alertResult;
         }
 
-        String[] cmd = {"/bin/sh", "-c", scriptPath + ALERT_TITLE_OPTION + "'" 
+ title + "'" + ALERT_CONTENT_OPTION + "'" + content + "'" + 
ALERT_USER_PARAMS_OPTION + "'" + userParams + "'"};
+        // avoid command injection (RCE vulnerability)
+        if (userParams.contains("'")) {
+            logger.error("shell script illegal user params : {}", userParams);
+            alertResult.setMessage("shell script illegal user params : " + 
userParams);
+            return alertResult;
+        }
+        if (title.contains("'")) {
+            logger.error("shell script illegal title : {}", title);
+            alertResult.setMessage("shell script illegal title : " + title);
+            return alertResult;
+        }
+        if (content.contains("'")) {
+            logger.error("shell script illegal content : {}", content);
+            alertResult.setMessage("shell script illegal content : " + 
content);
+            return alertResult;
+        }
+
+        String[] cmd = {"/bin/sh", "-c", scriptPath + ALERT_TITLE_OPTION + "'" 
+ title + "'" + ALERT_CONTENT_OPTION
+                + "'" + content + "'" + ALERT_USER_PARAMS_OPTION + "'" + 
userParams + "'"};
         int exitCode = ProcessUtils.executeScript(cmd);
 
         if (exitCode == 0) {
diff --git 
a/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/test/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptSenderTest.java
 
b/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/test/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptSenderTest.java
index 445d0738b5..64a811d474 100644
--- 
a/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/test/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptSenderTest.java
+++ 
b/dolphinscheduler-alert/dolphinscheduler-alert-plugins/dolphinscheduler-alert-script/src/test/java/org/apache/dolphinscheduler/plugin/alert/script/ScriptSenderTest.java
@@ -53,4 +53,12 @@ public class ScriptSenderTest {
         Assert.assertEquals("false", alertResult.getStatus());
     }
 
+    @Test
+    public void testScriptSenderInjectionTest() {
+        scriptConfig.put(ScriptParamsConstants.NAME_SCRIPT_USER_PARAMS, "' ; 
calc.exe ; '");
+        ScriptSender scriptSender = new ScriptSender(scriptConfig);
+        AlertResult alertResult = scriptSender.sendScriptAlert("test title 
Kris", "test content");
+        Assert.assertEquals("false", alertResult.getStatus());
+    }
+
 }