Is there a safer way to use system()?

Many security guidelines tell people not to use system() and similar functions, because the command is passed wholesale to the shell, and if the shell string isn't escaped properly, then you have all sorts of security problems: attackers can insert redirections to files (and file descriptors) they wouldn't otherwise have access to, and insert arbitrary commands by using ;, &, &&, ||, backticks, etc.

Yet, at the same time, system() is convenient: you don't have to build up your argument array by hand, you don't have to perform token parsing, variable substitution, or tilde expansion, and you don't have to do all the forking work. So, really, you just want all the conveniences of system() without the security risks.

Well, here's something that just might do the job, in certain instances where allowing users to specify command-line arguments is useful. Rather than passing the string to the shell, it uses wordexp() to do the parsing/substitution/expansion, which rejects all the unescaped shell metacharacters and backtick usages. I need to do more testing to be sure, but I am of the opinion that it's somewhat safer to use than system().

My code is based on the implementation of system() provided in the Single Unix Specification, and I release my modifications into the public domain.

#include <sys/wait.h>
#include <errno.h>
#include <signal.h>
#include <unistd.h>
#include <wordexp.h>

int
safer_system(const char *command)
{
    int result;
    struct sigaction sa_new, sa_oldintr, sa_oldquit;
    sigset_t ss_newblock, ss_oldblock;
    pid_t pid;
    wordexp_t we = {};

    if (!command)
        return 1;

    sa_new.sa_handler = SIG_IGN;
    sigemptyset(&sa_new.sa_mask);
    sa_new.sa_flags = 0;
    sigaction(SIGINT, &sa_new, &sa_oldintr);
    sigaction(SIGQUIT, &sa_new, &sa_oldquit);

    sigemptyset(&ss_newblock);
    sigaddset(&ss_newblock, SIGCHLD);
    sigprocmask(SIG_BLOCK, &ss_newblock, &ss_oldblock);

    switch (pid = fork()) {
    case -1:
        result = -1;
        break;

    case 0:
        sigaction(SIGINT, &sa_oldintr, NULL);
        sigaction(SIGQUIT, &sa_oldquit, NULL);
        sigprocmask(SIG_SETMASK, &ss_oldblock, NULL);
        if (wordexp(command, &we, WRDE_NOCMD)) {
            errno = EINVAL;
            return -1;
        }
        execvp(*we.we_wordv, we.we_wordv);
        _exit(127);
        /* NOTREACHED */

    default:
        while (waitpid(pid, &result, 0) == -1) {
            if (errno != EINTR) {
                result = -1;
                break;
            }
        }
    }

    sigaction(SIGINT, &sa_oldintr, NULL);
    sigaction(SIGQUIT, &sa_oldquit, NULL);
    sigprocmask(SIG_SETMASK, &ss_oldblock, NULL);
    return result;
}

My original version did the wordexp() call in the parent process, but this then expands $$ to the wrong process ID. There may be other bugs in this approach that I have yet to discover; as mentioned, I haven't done very much testing with it yet.