diff options
| author | Anant Patil <anant.patil@hp.com> | 2015-09-23 11:56:05 +0530 |
|---|---|---|
| committer | Zane Bitter <zbitter@redhat.com> | 2015-09-23 20:22:28 -0400 |
| commit | 6571e5ab646f55ba9411f5a668c5224203a08707 (patch) | |
| tree | fa69096cf8078f4897d7f949dd6b5b4960401922 | |
| parent | 5d5a2c1f221002586212299e3ba8799759d36af6 (diff) | |
| download | heat-cfntools-6571e5ab646f55ba9411f5a668c5224203a08707.tar.gz | |
Don't run commands given as list on shell
Commands from AWS::CloudFormation::Init, when supplied as list, should
be run with shell=False. Only when commands are given as string, they
are meant to be run on shell.
In principle, we are trying to give least access to the shell to avoid
any inadvertent shell injections.
Change-Id: I3dc6fe0c29a14f75be044846f737e1ade23a6d6b
Closes-Bug: 1498300
| -rw-r--r-- | heat_cfntools/cfntools/cfn_helper.py | 7 | ||||
| -rw-r--r-- | heat_cfntools/tests/test_cfn_helper.py | 24 |
2 files changed, 26 insertions, 5 deletions
diff --git a/heat_cfntools/cfntools/cfn_helper.py b/heat_cfntools/cfntools/cfn_helper.py index e241e57..c340bda 100644 --- a/heat_cfntools/cfntools/cfn_helper.py +++ b/heat_cfntools/cfntools/cfn_helper.py @@ -1100,11 +1100,8 @@ class CommandsHandler(object): if "command" in properties: try: command = properties["command"] - #FIXME bug 1498300 - if isinstance(command, list): - escape = lambda x: '"%s"' % x.replace('"', '\\"') - command = ' '.join(map(escape, command)) - command = CommandRunner(command, shell=True) + shell = isinstance(command, six.string_types) + command = CommandRunner(command, shell=shell) command.run('root', cwd, env) command_status = command.status except OSError as e: diff --git a/heat_cfntools/tests/test_cfn_helper.py b/heat_cfntools/tests/test_cfn_helper.py index 308ca03..06e6caf 100644 --- a/heat_cfntools/tests/test_cfn_helper.py +++ b/heat_cfntools/tests/test_cfn_helper.py @@ -1291,6 +1291,30 @@ class TestCfnInit(testtools.TestCase): md.cfn_init() mock_popen.assert_has_calls(calls) + @mock.patch.object(cfn_helper, 'controlled_privileges') + def test_cfn_init_runs_list_commands_without_shell(self, mock_cp): + calls = [] + returns = [] + # command supplied as list shouldn't run on shell + calls.extend(popen_root_calls([['/bin/command1', 'arg']], shell=False)) + returns.append(FakePOpen('Doing something')) + # command supplied as string should run on shell + calls.extend(popen_root_calls(['/bin/command2'], shell=True)) + returns.append(FakePOpen('All good')) + + md_data = {"AWS::CloudFormation::Init": {"config": {"commands": { + "00_foo": {"command": ["/bin/command1", "arg"]}, + "01_bar": {"command": "/bin/command2"} + }}}} + + with mock.patch('subprocess.Popen') as mock_popen: + mock_popen.side_effect = returns + md = cfn_helper.Metadata('teststack', None) + self.assertTrue( + md.retrieve(meta_str=md_data, last_path=self.last_file)) + md.cfn_init() + mock_popen.assert_has_calls(calls) + class TestSourcesHandler(testtools.TestCase): def test_apply_sources_empty(self): |