Are ALL system() calls a security risk in c++?

Question

A post in this (Are system() calls evil?) thread says:

Your program's privileges are inherited by its spawned programs. If your application ever runs as a privileged user, all someone has to do is put their own program with the name of the thing you shell out too, and then can execute arbitrary code (this implies you should never run a program that uses system as root or setuid root).

But system("PAUSE") and system("CLS") shell to the OS, so how could a hacker possibly intervene if it ONLY shells to a specific secure location on the hard-drive?

Does explicitly flush—by using fflush or _flushall—or closing any stream before calling system eliminate all risk?

Answer 1

The system function passes command to the command interpreter, which executes the string as an operating-system command. system uses the COMSPEC and PATH environment variables to locate the command-interpreter file CMD.exe. If command is NULL, the function just checks whether the command interpreter exists.

You must explicitly flush—by using fflush or _flushall—or close any stream before you call system.

https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/system-wsystem

In case, there are any doubts here's the actual snippet from the MS' implementation (very simple and straightforward):

// omitted for brevity
argv[1] = _T("/c");
argv[2] = (_TSCHAR *) command;
argv[3] = NULL;

/* If there is a COMSPEC defined, try spawning the shell */

/* Do not try to spawn the null string */
if (argv[0])
{
     // calls spawnve on value of COMSPEC vairable, if present
     // omitted for brevity
}

/* No COMSPEC so set argv[0] to what COMSPEC should be. */
argv[0] = _T("cmd.exe");

/* Let the _spawnvpe routine do the path search and spawn. */

retval = (int)_tspawnvpe(_P_WAIT,argv[0],argv,NULL);
// clean-up part omitted

As to concerns of what _tspawnvpe may actually be doing, the answer is: nothing magical. The exact invocation sequence for spawnvpe and friends goes as following (as anybody with licensed version of MSVC can easily learn by inspecting the spanwnvpe.c source file):

1. Do some sanity checks on parameters
2. Try to invoke _tspawnve on the passed file name. spawnve will succeed if the file name represents an absolute path to an executable or a valid path relative to the current working directory. No further checks are done - so yes, if a file named cmd.exe exists in current directory it will be invoked first in the context of system() call discussed.
3. In a loop: obtain the next path element using `_getpath()
   1. Append the file name to the path element
   2. Pass the resulted path to spwanvpe, check if it was successful

That's it. No special tricks/checks involved.

Answer 2

The original question references POSIX not windows. Here there is no COMSPEC (there is SHELL but system() deliberately does not use it); however /bin/sh is completely, utterly vulnerable.

Suppose /opt/vuln/program does system("/bin/ls"); Looks completely harmless, right? Nope!

$ PATH=. IFS='/ ' /opt/vuln/program

This runs the program called bin in the current directory. Oops. Defending against this kind of thing is so difficult it should be left to the extreme experts, like the guys who wrote sudo. Sanitizing environment is extremely hard.

So you might be thinking what is that system() api for. I don't actually know why it was created, but if you wanted to do a feature like ftp has where !command is executed locally in the shell you could do ... else if (terminalline[0] == '!') system(terminalline+1); else ... Since it's going to be completely insecure anyway there's no point in making it secure. Of course a truly modern use case wouldn't do it that way because system() doesn't look at $SHELL but oh well.