Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't set shell=True with untrusted input #26

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

benesch
Copy link

@benesch benesch commented Dec 21, 2021

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 #24, with a proposed fix in #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
[email protected] USER-FLAGS USER-COMMAND" and then passing that to the
local shell for interpretation, we instead build up a command array
like:

["ssh", "[email protected]", "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 #24.

Issue #, if available:

Description of changes:

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

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
[email protected] USER-FLAGS USER-COMMAND" and then passing that to the
local shell for interpretation, we instead build up a command array
like:

    ["ssh", "[email protected]", "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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mssh does not correctly handle commands written by Ansible
1 participant