akrennmair/newsbeuter

Rework settings that take commands as arguments

Minoru opened this issue · 1 comments

Our recent security vulnerabilities (#591, #598) are really caused by us using system(). As long as we pass some external input to that function, we're potentially vulnerable. The only winning move is not to play.

We can't just replace system() with fork() and exec() because we allow users to specify whole commands in some of the settings (pager, browser and so on.) Moving to exec() would mean we'd have to implement word-splitting and maybe some other shell features. We will still break some user's workflow because we won't be able/willing to implement the whole shell inside Newsbeuter.

It seems to be there's no other way than to break compatibility; we will limit commands to some simple stuff (just a path, or path with literal arguments—no variables, no subshells, no redirections.)

Alternatively we can just find an implementation of function that escapes arguments properly. Then we can keep using system() and stay confident that everything we pass there either came from the user (and thus assumed to be safe) or was received from the Internet and got escaped (and thus assumed to be safe).