From: Luis A. Garcia <luis@linux.vnet.ibm.com>
Date: Tue, 29 Oct 2013 18:44:12 +0000 (+0000)
Subject: Allow spaces in quoted SSH command arguments
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=2737c76cb2fb436f117a4f635aebca7a01691d88;p=openstack-build%2Fcinder-build.git

Allow spaces in quoted SSH command arguments

The check_ssh_injection() method was rejecting arguments with spaces
even when they were quoted, this was causing problems with some volume
driver commands such as commands for a storage pool with spaces in the
name.

Closes-Bug: #1244415
Change-Id: Ie4b809e1b39fdb752cf634e6d3c0a3924d8ac52b
---

diff --git a/cinder/tests/test_utils.py b/cinder/tests/test_utils.py
index fe1f54214..cef8d8553 100644
--- a/cinder/tests/test_utils.py
+++ b/cinder/tests/test_utils.py
@@ -456,16 +456,48 @@ class GenericUtilsTestCase(test.TestCase):
     def test_check_ssh_injection(self):
         cmd_list = ['ssh', '-D', 'my_name@name_of_remote_computer']
         self.assertIsNone(utils.check_ssh_injection(cmd_list))
+        cmd_list = ['echo', '"quoted arg with space"']
+        self.assertIsNone(utils.check_ssh_injection(cmd_list))
+        cmd_list = ['echo', "'quoted arg with space'"]
+        self.assertIsNone(utils.check_ssh_injection(cmd_list))
 
     def test_check_ssh_injection_on_error(self):
-        with_space = ['shh', 'my_name@      name_of_remote_computer']
-        with_danger_char = ['||', 'my_name@name_of_remote_computer']
+        with_unquoted_space = ['shh', 'my_name@      name_of_remote_computer']
         self.assertRaises(exception.SSHInjectionThreat,
                           utils.check_ssh_injection,
-                          with_space)
+                          with_unquoted_space)
+        with_danger_char = ['||', 'my_name@name_of_remote_computer']
         self.assertRaises(exception.SSHInjectionThreat,
                           utils.check_ssh_injection,
                           with_danger_char)
+        with_special = ['cmd', 'virus;ls']
+        self.assertRaises(exception.SSHInjectionThreat,
+                          utils.check_ssh_injection,
+                          with_special)
+        quoted_with_unescaped = ['cmd', '"arg\"withunescaped"']
+        self.assertRaises(exception.SSHInjectionThreat,
+                          utils.check_ssh_injection,
+                          quoted_with_unescaped)
+        bad_before_quotes = ['cmd', 'virus;"quoted argument"']
+        self.assertRaises(exception.SSHInjectionThreat,
+                          utils.check_ssh_injection,
+                          bad_before_quotes)
+        bad_after_quotes = ['echo', '"quoted argument";rm -rf']
+        self.assertRaises(exception.SSHInjectionThreat,
+                          utils.check_ssh_injection,
+                          bad_after_quotes)
+        bad_within_quotes = ['echo', "'quoted argument `rm -rf`'"]
+        self.assertRaises(exception.SSHInjectionThreat,
+                          utils.check_ssh_injection,
+                          bad_within_quotes)
+        with_multiple_quotes = ['echo', '"quoted";virus;"quoted"']
+        self.assertRaises(exception.SSHInjectionThreat,
+                          utils.check_ssh_injection,
+                          with_multiple_quotes)
+        with_multiple_quotes = ['echo', '"quoted";virus;\'quoted\'']
+        self.assertRaises(exception.SSHInjectionThreat,
+                          utils.check_ssh_injection,
+                          with_multiple_quotes)
 
     def test_create_channel(self):
         client = paramiko.SSHClient()
diff --git a/cinder/utils.py b/cinder/utils.py
index a92180b9d..de3d2481d 100644
--- a/cinder/utils.py
+++ b/cinder/utils.py
@@ -149,10 +149,21 @@ def check_ssh_injection(cmd_list):
     # Check whether injection attacks exist
     for arg in cmd_list:
         arg = arg.strip()
-        # First, check no space in the middle of arg
-        arg_len = len(arg.split())
-        if arg_len > 1:
-            raise exception.SSHInjectionThreat(command=str(cmd_list))
+
+        # Check for matching quotes on the ends
+        is_quoted = re.match('^(?P<quote>[\'"])(?P<quoted>.*)(?P=quote)$', arg)
+        if is_quoted:
+            # Check for unescaped quotes within the quoted argument
+            quoted = is_quoted.group('quoted')
+            if quoted:
+                if (re.match('[\'"]', quoted) or
+                        re.search('[^\\\\][\'"]', quoted)):
+                    raise exception.SSHInjectionThreat(command=str(cmd_list))
+        else:
+            # We only allow spaces within quoted arguments, and that
+            # is the only special character allowed within quotes
+            if len(arg.split()) > 1:
+                raise exception.SSHInjectionThreat(command=str(cmd_list))
 
         # Second, check whether danger character in command. So the shell
         # special operator must be a single argument.