1

I am trying to find if the following shell script is vulnerable to command injection

#!/bin/sh

set -x
dig +noall +answer TXT $2._domainkey.$1

Now when I try something like this,

sh script.sh "sparkpost.com & echo \$(whoami)" "google"

(Note: the script is actually executed by a C program using posix_spawn)

the following output is printed

+ dig +noall +answer TXT google._domainkey.sparkpost.com & echo $(whoami)
...
...

The echo command is NOT executed.

Can the input be modified to make the injection possible or is it that position parameters aren't vulnerable?

Krishnaraj
  • 163
  • 4
  • 1
    Exactly how are you executing this with `posix_spawn`? If you construct a command string and pass that to a shell to parse, you almost certainly are vulnerable. If you pass arguments to the script directly, you probably aren't vulnerable to *command* injection, but are vulnerable to shell word splitting and wildcard expansion trickery. (To avoid that, double-quote your parameter references! [Shellcheck.net](https://www.shellcheck.net) is your friend here.) – Gordon Davisson Feb 28 '21 at 01:03

2 Answers2

2

I see three layers of interpretation where there might be some sort of injection vulnerability.

(EDIT: immediately after posting this, I realized that since you're working with domain names, you could just validate the inputs in C before any of this, and make the whole thing moot. The allowed characters in domain names -- plain-ASCII letters and digits, dash (but not as the first character), and period -- are restrictive enough that you'd completely block all of these injection opportunities.)

Layer 1: There might be a command injection vulnerability in how you pass the arguments to posix_spawn. Fortunately, it sounds like you don't use it the vulnerable way, but I'll discuss it for completeness.

If you construct a command string (that includes untrusted data) like sh script.sh "<untrusted data here>" "<more untrusted data>", and pass that via posix_spawn to a shell for interpretation, something like this:

char *argv[] = {"sh", "-c", cmdStringWithUntrustedData, NULL};
status = posix_spawn(&pid, "/bin/sh", NULL, NULL, argv, environ);

...then you're almost certainly vulnerable to command injection. There are ways to safely quote/escape untrusted data for inclusion in a command string, but it's tricky to get right and you're better off avoiding it.

(BTW, my C is pretty rusty; please ignore any errors.)

The safe way to do this is to avoid the extra level of shell interpretation, and pass the arguments (as separate strings) directly to the shell that'll be running the script:

char *argv[] = {"sh", "script.sh", untrustedData1, untrustedData2, NULL};
                 /* ^^^ note the lack of "-c" */
status = posix_spawn(&pid, "/bin/sh", NULL, NULL, argv, environ);

Since the script has a shebang, if it's also executable you could also safely execute it directly, without specifying a shell:

char *argv[] = {"script.sh", untrustedData1, untrustedData2, NULL};
status = posix_spawn(&pid, "/path/to/script.sh", NULL, NULL, argv, environ);

With these methods, the untrusted strings are passed to the script without any interpretation, so they're safe from injection problems (at least until the script starts using them).

Layer 2: There's an injection vulnerability when the script uses the arguments. If parameter/variable/etc references are not enclosed in double-quotes, then values are subject to word splitting (basically, being broken up into separate strings on whitespace) and filename wildcard expansion. This isn't strictly a command injection, but it does open up a variety of opportunities for an attacker to trigger unexpected argument interpretation, do filesystem enumeration, etc. So replace this:

dig +noall +answer TXT $2._domainkey.$1

with this:

dig +noall +answer TXT "$2._domainkey.$1"

Note that the ._domainkey. part could be double-quoted (as above) or not (as in "$2"._domainkey."$1") -- since it doesn't contain any special characters, it doesn't matter. Also, you might think that an attacker could escape from the double-quotes by e.g. including a double-quote in the argument, but that's not the case; when a parameter or variable reference occurs inside double-quotes, it's expanded and then no further processing is done on the expanded value. So no quote/escape/etc trickery is possible here.

BTW, there are some contexts where it's safe to leave off the double-quotes, but IMO it's easiest and safest to just always double-quote them.

Layer 3: There's an option injection vulnerability with the dig command's interpretation of its arguments. Most commands treat arguments that start with "-" as options (/flags) rather than normal arguments; dig also treats arguments that start with "+" as query options, and those that start with "@" as a servername to query. Double-quotes have no effect on this, so when an argument starts with untrusted data, you need to be careful that this doesn't create a vulnerability.

In the case of dig, I don't see any obvious way to do anything really nasty here, but it's best to sanitize your inputs anyway. You can do this for each parameter with a case statement. Here's an example that restricts them to valid domain name characters:

case "$1" in
    [-+@]* )    # Forbidden as first character
        # Maybe log/report that someone's trying option injection?
        exit 1 ;;
    *[!-.A-Za-z0-9]* )    # Only allowed chars at *any* position
        # maybe report/log an error here?
        exit 1 ;;
esac

case "$2" in
    [-+@]* )    # Forbidden as first character
        # Maybe log/report that someone's trying option injection?
        exit 1 ;;
    *[!-.A-Za-z0-9]* )    # Only allowed chars at *any* position
        # maybe report/log an error here?
        exit 1 ;;
esac

If you wanted to be really picky, you could disallow ".-" in the middle of a domain string, since "-" is forbidden at the beginning of a domain label (but I don't think that's relevant for security). Also, if you can count on bash, and not some other shell, you could use its regex-matching feature instead of glob patterns in a case statement. But you're using a /bin/sh shebang, so that's not safe.

Gordon Davisson
  • 2,581
  • 1
  • 17
  • 13
0

Yes it can!

Like SQL injection, the key is to break the command sequence and insert your own.

In this case, a semi-colon ";" will perform the break.

Try this against your script:

sh script.sh "sparkpost.com ";whoami; echo "google";ls
user10216038
  • 7,552
  • 2
  • 16
  • 19
  • 1
    This depends on the command (`sh script ...`) being parsed by a shell before being executed, in which case the shell parses the `;` as a command delimiter *before* passing it as an argument to the script. If `posix_spawn` is called like `char *argv[] = {"script.sh", "sparkpost.com ; whoami; echo", "google", NULL}; status = posix_spawn(&pid, "/bin/sh", NULL, NULL, argv, environ);` then the initial command string is never parsed by a shell, the `;` is just part of the argument, and the command injection does not happen. – Gordon Davisson Feb 28 '21 at 00:58
  • @Gordon Davisson - An excellent point. Filtering user input can be tricky and needs careful evaluation. – user10216038 Feb 28 '21 at 01:05
  • @GordonDavisson Yes, `posix_spawn` is used exactly like the way you have described so I guess its safe. Can you please add this as an answer? – Krishnaraj Feb 28 '21 at 07:19