How to re-write this function to avoid argument injection

Question

I have a function in my .bashrc file that allows me to run a script on a remote server with arguments via ssh.

Currently, the function contains:

function runMyScript {
    if [ $1 = "s3" ]
    then
        ssh -i "~/path/to/.pem" server.amazonaws.com "/home/ubuntu/scripts/script.sh ${1} ${2} ${3}"
    elif [ $1 = "ec2" ]
    then 
        ssh -i "~/path/to/.pem" server.amazonaws.com "/home/ubuntu/scripts/script.sh ${1} ${2}"
    else
        echo "** Run with s3 or ec2 options"
    fi
}

So, I would be able to call the function, with either runMyScript arg_1 arg_2 arg_3 or runMyScript arg_1 arg_2.

How to re-write this function to make it more secure and avoid possible argument injection?

Answer 1

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.

Answer 2

As a complement to @ikkachu's fine answer:

Compared to passing arbitrary strings to eval or sh -c, the issue is aggravated here by:

• the fact that you don't know what shell will be used on the remote host to parse that command line.
• What the locale will be, and in particular what charset will be used on the remote host. Whether it's the same as on the local system or not.

Your question is mostly a subset of How to execute an arbitrary simple command over ssh without knowing the login shell of the remote user?

So the answers there will apply here.

In particular if sshd on the remote host allows passing environment variables that start with LC_* (as many openssh deployments do), you can do:

LC_ARG1=$1 LC_ARG2=$2 LC_ARG3=$3 ssh -o SendEnv='LC_*' host exec sh -c \
   \''exec my-script "$LC_ARG1" "$LC_ARG2" "$LC_ARG3"'\'

Sanitizing the input is a good idea, but note that you don't want to use ranges like [A-Z] unless you're in the C locale. For instance, in most GNU system locales, Ǒ is in that range, and for instance in the zh_HK.big5hkscs locale, that character is encoded as 0x88 0x60 and you may recognise 0x60 as the ASCII (and BIG5HKSCS) encoding of the backtick character!

Best is to list the allowed characters individually:

is_safe() case $1 in
  *[!-_.ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz]*) false;;
esac

Also beware of the empty string, which will need to be quoted ('' is supported by all common shells). And beware of arguments starting with - that some tools might take as options.