From 2737c76cb2fb436f117a4f635aebca7a01691d88 Mon Sep 17 00:00:00 2001 From: "Luis A. Garcia" Date: Tue, 29 Oct 2013 18:44:12 +0000 Subject: [PATCH] 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 --- cinder/tests/test_utils.py | 38 +++++++++++++++++++++++++++++++++++--- cinder/utils.py | 19 +++++++++++++++---- 2 files changed, 50 insertions(+), 7 deletions(-) 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[\'"])(?P.*)(?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. -- 2.45.2