Shell – How to re-write this function to avoid argument injection

argumentsquotingSecurityshellssh

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?

Best Answer

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.

Related Question