How to avoid possible argument injection?
Find out what sorts of inputs you want to pass through, and make sure the arguments only contain those.
In the least, make sure they don't contain characters special to the remote shell.
$1
is not much of a problem since you compare it against known values. Though you need to double-quote it, otherwise it may expand to multiple words in the comparison, and that may allow passing funny values through it (something like 1 -o x
would pass the comparison).
Use if [ "$1" = "s3" ] ; then ...
instead.
As for $2
and $3
, passing them through ssh
is basically the same as passing them to eval
or sh -c
. Any substitutions inside them will be expanded at the remote.
Say we'll run ssh somehost "echo $var"
. Now if var
contains $(ls -ld /tmp)
, the remote command line will be echo $(ls -ld /tmp)
, and the ls
is executed on the remote. Double-quoting the variable will not help with command expansion, but single quotes would. With the command written as
ssh somehost "echo '$var'"
, the command expansion does not happen.
Single-quotes inside the variable are still a problem, as they will terminate the single-quoting, so in the least, we'll need to check for that:
case "$var" in *"'"*) echo var has a single-quote; exit 1 ;; esac
Though we might as well check for any special characters we don't want to pass through. Dollar signs start most of the expansions, backticks start command expansion, and quotes I don't trust either:
case "$var" in *[\'\"\$\`]*) echo var has something funny; exit 1 ;; esac
But I'm not sure if that's all, so better just whitelist the characters we want to pass through. If letters, digits, dashes and underscores are enough:
case "$var" in *[!-_a-zA-Z0-9]*) echo var has something funny; exit 1 ;; esac
So, this might be a start:
function runMyScript {
case "$2" in *[!-_a-zA-Z0-9]*) exit 1 ;; esac
case "$3" in *[!-_a-zA-Z0-9]*) exit 1 ;; esac
if [ "$1" = "s3" ] ; then
ssh somehost "script.sh '$1' '$2' '$3'"
elif [ "$1" = "ec2" ] ; then
ssh somehost "script.sh '$1' '$2'"
else
echo "** Run with s3 or ec2 options"
fi
}
Noting that you used function runMyScript
instead of runMyScript()
, you're not running a plain POSIX shell. If it happens to be Bash, you could rewrite the pattern matches with the [[...]]
test, which supports regular expression matches.
Newish versions of Bash also have the ${var@Q}
expansion, which should produce the contents of var
quoted in a format that can be reused as input. It might also be of use, but I don't have a new enough bash at hand to test it.
Also, don't blindly trust me to remember all the possible quirks of the shell language.
script.sh
doing. – Apr 02 '17 at 21:25