Unvanquished / updater

QML based updater to install, update and launch the Unvanquished game.
https://unvanquished.net/download
16 stars 7 forks source link

prevent people "accidentally" run updater as root #65

Closed ghost closed 3 years ago

ghost commented 3 years ago

I've seen on IRC that some people run the updater as root, and then chmod foo:bar $GAMEDIR. Running stuff as root can be handful, but also reserved to people which know what and why they're doing it, so, on linux and bsd systems, I would check against geteuid() == 0 (or getuid, in case someone gave the updater the setuid bit, unlikely, but code is the same and this feels safer to me?)

slipher commented 3 years ago

So the case we especially need to avoid is when the user runs as root, uses the default install location, and ~/.local/share/unvanquished does not already exist. This results in the homepath being owned by root, so the game is not usable.

I think what I'll do is, when the user clicks "start download", to test if the install location is inside $HOME (a condition more general and future-proof than looking specifically for the homepath). If so, tell the user to restart as non-root and close the program.

It would also be nice to ensure that Unix permission bits include world-readable and -executable if an install as root is done somewhere else (I'm hoping this will just be inherited from the parent directory and we wouldn't have to do anything).

DolceTriade commented 3 years ago

Can we just yell at people running as root and complain or at least make them work a little harder asking if this is something they want to do?

if (getuid() == 0) { // complain }

illwieckz commented 3 years ago

to test if the install location is inside $HOME

This is not a good idea. The updater follows the XDG specification allowing anyone to replace ~/.local/share/ with the $XDG_DATA_HOME environment variable. It's very OK to store data outside $HOME (like, on a NAS). Also, $HOME may not be stored in /home.

The default Unvanquished home directory on Linux is: ${XDG_DATA_HOME:-${HOME}/.local/share}/unvanquished, this can be anywhere.

If we really want some warning and things like that, I would prefer to require the user to pass a special argument like --yes-i-know-what-i-am-doing and abort (with a warning) otherwise if running with pid 0, or something like that.

If updater is running as root (pid 0), we may want to install to /opt/Unvanquished. When running the game as user the home dir would still be ${XDG_DATA_HOME:-${HOME}/.local/share}/unvanquished and then, writable. We would also have to install the .desktop file widely.

slipher commented 3 years ago

I think you misunderstood something. I never suggested the user should be forced to use a path inside $HOME. The condition I proposed for telling the user they must use a different path is that the updater is running as root AND they picked a path inside $HOME.

illwieckz commented 3 years ago

Hum, how would you know the user's $HOME if you're root? $HOME will be /root for root, which is not available to the user.

Right, there is some still-broken distros out there giving non-root $HOME environment variable to root user when using sudo, but this is a mistake of the distro which can break more than Unvanquished (bash history file, vim temporary files, X11-over-ssh, user cache, even prevent the user to login later…). Are we talking about that issue from distributions doing sudo the wrong way?

slipher commented 3 years ago

I did not know changing the environment variables is a functionality of sudo. I thought sudo is not supposed to run the profile script or anything, and keep the same environment of the calling user. I find on Mac the $HOME of the calling user is kept, but on Debian it changes as you say.

So instead of that, I may have it refuse to install if the user left the default install directory of /base.