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
This commit is contained in:
Anant Patil 2015-09-23 11:56:05 +05:30 committed by Zane Bitter
parent 5d5a2c1f22
commit 6571e5ab64
2 changed files with 26 additions and 5 deletions

View File

@ -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:

View File

@ -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):