Comment by deathanatos
11 hours ago
> well-designed to be secure
While I suppose this doesn't say the design isn't secure, system() is one of those calls that has no place in modern code. It is insecure by its design.
> Any user input that is employed as part of `command` should be carefully sanitized, to ensure that unexpected shell commands or command options are not executed. Such risks are especially grave when using system() from a privileged program.
(—man 3 system)
The "system" function (and also "popen" function) are helpful, although they should be used carefully. You should only use it where the entire input (rather than merely a part of it) comes from a trusted source from the local user (and documented in a clear way that it does this), such as being entered interactively or from a user configuration file, or in some cases a entirely hard-coded string (although in such a case, often one of the exec functions works better), and to ensure that the security boundary is correct (e.g. you should probably avoid it if it is setuid). If it is necessary to pass additional data then you might use environment variables, pipes, temporary files, etc.
Strings passed to system or popen should not be constructed by combining other strings; they should be directly unchanged from whatever trusted source it comes from.
The specific use documented in the article is a situation where I should think that you should not call the shell (since the command includes untrusted input, and also because there might be a better way to display the error message).
There are additional possible security issues with such things though, whether you use the shell or execute directly, some of which are due to the use of text rather than binary data for communication (although changing that won't solve everything).
Well, so there's two other problems with system (and popen):
1. IME security teams want to run dumb linters over the code that look for such things. While uses such as the ones you describe are secure, the linter might be blunter than that. I'm not sure that's necessarily a bad thing: "this use of system(3) is secure" has a cognitive tax at review time, and as the code changes; often, I prefer the stance of "don't make me think" with regards to security: i.e., do the simple, trivially secure thing, not the complex, secure under the just-right conditions thing. Then we don't have to persuade (potentially non-technical) security teams, non-technical auditors, linters, etc. that are going over the code with blunt instruments.
2. (And much more minor) the entire execution of sh is often just wasted performance that an exec(2) removes.
(& yes, you are also right that even an exec(2), mishandled, can have its problems in some circumstances. But typically when the pattern is system("<shell that just runs what we'd exec(2)>"), then whatever those problems are, system(3) is going to share them.)