waycrate / swhkd

Sxhkd clone for Wayland (works on TTY and X11 too)
https://git.sr.ht/~shinyzenith/swhkd
BSD 2-Clause "Simplified" License
676 stars 47 forks source link

[fix] raise swhkd privileges right after reading config #156

Closed ajanon closed 1 year ago

ajanon commented 2 years ago

As a fix for CVE-2022-27814, root privileges are dropped to the calling user when (re)loading the config file. Privileges were sometimes dropped but never raised again, which caused crashes when sending SIGHUP to swhkd multiple times in a row.

This now always raises privileges after successfully reading the config file. Fixes #155.

Shinyzenith commented 2 years ago

@EdenQwQ Hello dear, would you mind reviewing this?

Shinyzenith commented 2 years ago

So my only concern is that we never drop the privileges in this patch after raising them.

ajanon commented 2 years ago

Privileges are dropped right at the beginning of the load_config closure: https://github.com/waycrate/swhkd/blob/022466ec0bcca881d5034a2b23aab934cfd4578a/swhkd/src/daemon.rs#L72-L74

With this PR the daemon has full privileges, except while loading the config. If you want to restrict it further, I can try to implement the fix for SIGHUP another way.

Shinyzenith commented 2 years ago

Privileges are dropped right at the beginning of the load_config closure:

https://github.com/waycrate/swhkd/blob/022466ec0bcca881d5034a2b23aab934cfd4578a/swhkd/src/daemon.rs#L72-L74

With this PR the daemon has full privileges, except while loading the config. If you want to restrict it further, I can try to implement the fix for SIGHUP another way.

This looks good enough! I'll just perform some security tests on my end once I get the time and merge! ❤️

Shinyzenith commented 1 year ago

To git@github.com:waycrate/swhkd 6b24df8..36281fe main -> main