Wrap command in shlex.quote() #25
Conversation
|
Oof, yeah, I just ran into exactly the same issue! I'm not sure who at AWS needs to take a look at this, but this is a huge footgun with I"m not entirely convinced this is the right solution though. That |
Previously `mssh` would blindly execute an SSH command, resulting in
shell pipelines being executed on the *host* rather on the SSH target.
Consider the following command:
$ mssh i-04bb8a432b18b2250 'whoami; whoami'
ubuntu
benesch
The second invocation of "whoami" runs on the host and therefore prints
my local username, rather than the username on the EC2 instance.
This is at odds with the normal SSH program, which would print "ubuntu"
for both, as any shell metacharacters are left to be interpreted by the
remote shell.
This issue was previously reported as aws#24, with a proposed fix in aws#25
that simply shell quotes the command. That solution seems suboptimal to
me, as it is generally a bad idea to pass user input to a shell.
This commit solves the issue another way, by keeping track of individual
arguments as we go. Rather than building up a command string like "ssh
ubuntu@10.0.0.1 USER-FLAGS USER-COMMAND" and then passing that to the
local shell for interpretation, we instead build up a command array
like:
["ssh", "ubuntu@10.0.0.1", "USER-FLAG-1", "USER-FLAG-2", "USER-COMMAND"]
This command can be executed without invoking the shell, and so we can
be sure it will not execute any code on the host.
Fix aws#24.
Done in #26. |
|
@hyandell Could you (or someone from AWS) please merge this PR (or better #26)? Forgotten quoting is a very, very dangerous thing. It may result into security holes or even deletion of local files: imagine someone runs: This command will delete the LOCAL directory |
|
I agree. I reported this to AWS as a security bug earlier this year but they demurred.
|
3 participants
Issue #, if available: #24
Description of changes:
This updates
get_command()by wrapping the command in shlex.quote().As seen in the linked issue (#24) the mssh command has trouble executing commands that contain escaped quotes
'such as those generated automatically by Ansible. These quotes can be meaningful and may cause errors if omitted. Wrapping the whole command in shlex.quote preserves the original quotes and solves the issues.I updated the test cases to expect the quotes in the command. As far as I can tell, this shouldn't break anything but I'm open to thoughts or concerns. It may also be slightly more secure than before, not that you should be running un-trusted commands through ssh anyways
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.