From 619bd1e876ac21f691dbcf9df7cd1b80311432de Mon Sep 17 00:00:00 2001
From: Billie Rinaldi <billie@apache.org>
Date: Thu, 5 Sep 2019 12:49:16 -0700
Subject: [PATCH] YARN-9718. Fixed yarn.service.am.java.opts shell injection.
 Contributed by Eric Yang

(cherry picked from commit 2e2e5401f297545181323b126a69eaa2239afb02)
---
 .../hadoop/yarn/service/client/ServiceClient.java    |  3 +++
 .../service/exceptions/RestApiErrorMessages.java     |  1 +
 .../hadoop/yarn/service/utils/ServiceApiUtil.java    | 12 ++++++++++++
 .../yarn/service/utils/TestServiceApiUtil.java       |  6 ++++++
 4 files changed, 22 insertions(+)

diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/client/ServiceClient.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/client/ServiceClient.java
index f57088c0dbc..7d3903a8476 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/client/ServiceClient.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/client/ServiceClient.java
@@ -1049,6 +1049,9 @@ public class ServiceClient extends AppAdminClient implements SliderExitCodes,
       jvmOpts += DEFAULT_AM_JVM_XMX;
     }
 
+    // validate possible command injection.
+    ServiceApiUtil.validateJvmOpts(jvmOpts);
+
     CLI.setJVMOpts(jvmOpts);
     if (hasSliderAMLog4j) {
       CLI.sysprop(SYSPROP_LOG4J_CONFIGURATION, YARN_SERVICE_LOG4J_FILENAME);
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/exceptions/RestApiErrorMessages.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/exceptions/RestApiErrorMessages.java
index 57c6449a982..295f14af098 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/exceptions/RestApiErrorMessages.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/exceptions/RestApiErrorMessages.java
@@ -127,4 +127,5 @@ public interface RestApiErrorMessages {
       " not contain a hostname.";
   String ERROR_KERBEROS_PRINCIPAL_MISSING = "Kerberos principal or keytab is" +
       " missing.";
+  String ERROR_JVM_OPTS = "Invalid character in yarn.service.am.java.opts.";
 }
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/utils/ServiceApiUtil.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/utils/ServiceApiUtil.java
index 28cb07af2b2..d1120248504 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/utils/ServiceApiUtil.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/main/java/org/apache/hadoop/yarn/service/utils/ServiceApiUtil.java
@@ -61,6 +61,8 @@ import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
 
 import static org.apache.hadoop.yarn.service.exceptions.RestApiErrorMessages.ERROR_COMP_DOES_NOT_NEED_UPGRADE;
 import static org.apache.hadoop.yarn.service.exceptions.RestApiErrorMessages.ERROR_COMP_INSTANCE_DOES_NOT_NEED_UPGRADE;
@@ -240,6 +242,16 @@ public class ServiceApiUtil {
     }
   }
 
+  public static void validateJvmOpts(String jvmOpts)
+      throws IllegalArgumentException {
+    Pattern pattern = Pattern.compile("[!~#?@*&%${}()<>\\[\\]|\"\\/,`;]");
+    Matcher matcher = pattern.matcher(jvmOpts);
+    if (matcher.find()) {
+      throw new IllegalArgumentException(
+          RestApiErrorMessages.ERROR_JVM_OPTS);
+    }
+  }
+
   public static void validateKerberosPrincipal(
       KerberosPrincipal kerberosPrincipal) throws IOException {
     try {
diff --git a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/test/java/org/apache/hadoop/yarn/service/utils/TestServiceApiUtil.java b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/test/java/org/apache/hadoop/yarn/service/utils/TestServiceApiUtil.java
index e36860e3dbd..d1a7f9f1288 100644
--- a/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/test/java/org/apache/hadoop/yarn/service/utils/TestServiceApiUtil.java
+++ b/hadoop-yarn-project/hadoop-yarn/hadoop-yarn-applications/hadoop-yarn-services/hadoop-yarn-services-core/src/test/java/org/apache/hadoop/yarn/service/utils/TestServiceApiUtil.java
@@ -718,6 +718,12 @@ public class TestServiceApiUtil extends ServiceTestUtils {
     }
   }
 
+  @Test(expected = IllegalArgumentException.class)
+  public void testJvmOpts() {
+    String jvmOpts = "`ping -c 3 example.com`";
+    ServiceApiUtil.validateJvmOpts(jvmOpts);
+  }
+
   public static Service createExampleApplication() {
 
     Service exampleApp = new Service();