Skip to content
This repository has been archived by the owner on Oct 10, 2019. It is now read-only.

Rework settings that take commands as arguments #599

Open
Minoru opened this issue Sep 16, 2017 · 1 comment
Open

Rework settings that take commands as arguments #599

Minoru opened this issue Sep 16, 2017 · 1 comment

Comments

@Minoru
Copy link
Collaborator

Minoru commented Sep 16, 2017

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.)

@Minoru Minoru added this to the 3.0 milestone Sep 16, 2017
@Minoru Minoru changed the title Reword settings that take commands as arguments Rework settings that take commands as arguments Sep 16, 2017
@Minoru
Copy link
Collaborator Author

Minoru commented Sep 17, 2017

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).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

1 participant