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.