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:
parent
5d5a2c1f22
commit
6571e5ab64
|
@ -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:
|
||||
|
|
|
@ -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):
|
||||
|
|
Loading…
Reference in New Issue