Merge pull request #4675 from StackStorm/service_action_fix

Fix a possible shell command injection in the linux.service action

Author: Kami

CHANGELOG.rst

@@ -30,6 +30,11 @@ Fixed
   In such scenario, package / module was incorrectly loaded from Python 2 site-packages instead of
   Python 3 standard library which broke such packs. (bug fix) #4658 #4674
 * Remove policy-delayed status to avoid bouncing between delayed statuses. (bug fix) #4655
+* Fix a possible shell injection in the ``linux.service`` action. User who had access to run this
+  action could cause a shell command injection by passing a compromised value for either the
+  ``service`` or ``action`` parameter. (bug fix) #4675
+
+  Reported by James Robinson (Netskope and Veracode).
 
 3.0.0 - April 18, 2019
 ----------------------

contrib/linux/actions/service.py

@@ -20,20 +20,25 @@
 import platform
 import subprocess
 
+from st2common.util.shell import quote_unix
+
 distro = platform.linux_distribution()[0]
 
-args = {'act': sys.argv[1], 'service': sys.argv[2]}
+if len(sys.argv) < 3:
+    raise ValueError('Usage: service.py <action> <service>')
+
+args = {'act': quote_unix(sys.argv[1]), 'service': quote_unix(sys.argv[2])}
 
 if re.search(distro, 'Ubuntu'):
     if os.path.isfile("/etc/init/%s.conf" % args['service']):
-        cmd = args['act'] + " " + args['service']
+        cmd_args = ['service', args['service'], args['act']]
     elif os.path.isfile("/etc/init.d/%s" % args['service']):
-        cmd = "/etc/init.d/%s %s" % (args['service'], args['act'])
+        cmd_args = ['/etc/init.d/%s' % (args['service']), args['act']]
     else:
         print("Unknown service")
         sys.exit(2)
 elif re.search(distro, 'Redhat') or re.search(distro, 'Fedora') or \
         re.search(distro, 'CentOS Linux'):
-    cmd = "systemctl %s %s" % (args['act'], args['service'])
+    cmd_args = ['systemctl', args['act'], args['service']]
 
-subprocess.call(cmd, shell=True)
+subprocess.call(cmd_args, shell=False)

contrib/linux/actions/service.yaml

@@ -7,7 +7,7 @@
   parameters:
     act:
       type: "string"
-      description: "Action to perform on service"
+      description: "Action to perform on service (e.g. start, stop, restart, etc.)"
       position: 0
     service:
       type: "string"