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.