Passing Strings to the Shell

XXX: shell: shell metas; command invocation: leading dashes

This is another variation of the string sanitizing issue discussed above, but since it is a pretty common task (and the source of a plethora of security problems) I feel we need to go into this in a lot more detail.

My first advice is, avoid calling external programs if you can. If you can't, at least try to avoid using the shell. We've already discussed the use of fork/exec in chapter 3, but let's take a look at another example here.

Consider an email client that is smart enough to detect links to web pages embedded in emails. When it sees a string that looks like a URL (such as http://www.foo.com), it allows the user to click on that link and fire up his preferred web browser and display that page.

The naive approach is to do this:

    char        command[1024];

    snprintf(command, sizeof(command), "netscape '%s' &", url);
    system(command);

This is quite deadly. At first glance, this may look safe because the programmer put the URL between single quotes, and of course the shell ignores all special characters when they occur between single quotes. All characters except the single quote, that is. Invoking the code above with a URL like http:'&rm -rf ~;' will quite nicely wipe the victim's home directory because it will invoke netscape in the background with an argument of http:, and run rm recursively on the user's home directory.^8.2

Now all it takes an attacker is to send you a message containing one of these very long URLs with lots of funny characters in them that no-one bothers to check anyway because they immediately give you a headache, and ask you to view it. Oops, there goes the 700 page family history that took you four years to write.

Given this problem, a programmer may decide to simply remove all occurences of the single quote character from the URL, and think he's safe. If the URL doesn't contain single quotes anymore, then the shell should ignore all meta characters in the string?

It should, but it won't necessarily do so. For instance, some versions of bash would interpret the character with decimal code 255 as the line separator, i.e. pretty much the same as a semicolon, except that it was being honored even when found within single quotes!

Therefore, avoid the shell in general like the plague, and system and popen in particular! As mentioned above, the Right Thing is to use the fork and exec system calls. It's a bit of extra coding, but it is definitely more secure as it doesn't involve the shell anymore. For instance, the URL displaying code quoted above could be rewritten like this

static char *   url_viewer = "netscape -remote openURL(%s)";

void
execute_viewer(const char *url)
{
    char    buffer[1024], *command, *tok;
    char    *argv[16];
    int     n = 0, pid;

    if (!sane_url(url))
        return;
    if ((pid = fork()) < 0) {
        perror("fork");
        return;
    }
    if (pid) {
        /* Parent process: return immediately, don't wait for viewer
         * to finish */
        return;
    }

    command = strdup(url_viewer);
    tok = strtok(command, " \t");
    while (tok) {
        if (strstr(tok, "%s")) {
            snprintf(buffer, sizeof(buffer), tok, url);
            tok = strdup(buffer);
        }
        argv[n++] = tok;
        tok = strtok(NULL, " \t");
    }
    argv[n++] = NULL;

    execvp(argv[0], argv);
    perror(argv[0]);
    exit(1);
}

Note that the code assumes that url_viewer is specified in the application's configuration, which is trusted. Otherwise we'd have to be more careful (e.g. with the number of tokens into which the command is split up, as to not overflow the argv array, and the format string passed to snprintf).

Also note the use of the execvp function, which is plain evil in setuid programs as it uses the environment variable PATH to locate the external program. In a network application context, execvp is usually okay as long as the untrusted peer cannot modify your environment.