Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unsafe Installation #102

Closed
serverwentdown opened this issue Mar 25, 2020 · 6 comments
Closed

Unsafe Installation #102

serverwentdown opened this issue Mar 25, 2020 · 6 comments

Comments

@serverwentdown
Copy link

serverwentdown commented Mar 25, 2020

This is a really important security issue. I think we should change the installer to install cpufreqctl into /usr/local/bin/.

Allowing users to run scripts installed in the user folder is completely unsafe, effectively providing root privileges to any program running as the current user.

Example exploit, run as the user:

mv $HOME/.local/share/gnome-shell/extensions/cpupower@mastercaution/src/cpufreqctl $HOME/old # Succeeds because the directory is user writeable
echo -e '#!/bin/sh\ntouch /root/pwned\necho you are $(whoami)' > $HOME/.local/share/gnome-shell/extensions/cpupower@mastercaution/src/cpufreqctl
chmod +x $HOME/.local/share/gnome-shell/extensions/cpupower@mastercaution/src/cpufreqctl
pkexec $HOME/.local/share/gnome-shell/extensions/cpupower@mastercaution/src/cpufreqctl

Instead, the installer should drop cpufreqctl into /usr/local/bin or rely on a system program.

These lines attempted to fix the issue, but directories that are writeable can have its files renamed.

This also happens to fix #95 😄

@mastercaution
Copy link
Contributor

Very good that you spotted it, it's very important that we fix this!

Moving cpufreqctl to /usr/local/bin is a good idea in my eyes, but it means, that cpufreqctl will be installed for all users.
So we also have to work on #84 for customizable multi-user support ... if this is needed.

@fin-ger
Copy link
Member

fin-ger commented Mar 29, 2020

The current idea is to add a Makefile to the project with the following targets:

install # installs extension to $PREFIX/share/gnome-shell/extensions
uninstall # uninstalls extension from $PREFIX/share/gnome-shell/extensions
install-tool # installs cpufreqctl to $PREFIX/bin and polkit
uninstall-tool # uninstalls cpufreqctl from $PREFIX/bin and polkit

extensions.gnome.org workflow

Install it via extensions.gnome.org, where the extension will install the cpufreqctl tool to /usr/loca/bin/cpufreqctl-$userid and sets up a polkit rule. This is necessary as different users might use different versions of cpupower.

Admins remote install per user workflow

Login to the remote machine and run make install install-tool PREFIX=/home/worker/.local TOOL_SUFFIX=worker will install the extension to /home/worker/.local/share/gnome-shell/extensions/ and the cpufreqctl tool to /usr/local/bin/cpufreqctl-worker and sets up a polkit rule for /usr/local/bin/cpufreqctl-worker for user worker.

Admins remote install global workflow

Login to the remote machine and run make install install-tool PREFIX=/usr will install the extension to /usr/share/gnome-shell/extensions and the cpufreqctl tool to /usr/bin/cpufreqctl and sets up a polkit rule for /usr/bin/cpufreqctl for normal users (maybe for the wheel group?).

Package maintainers workflow

Same as admins remote install global workflow.

Developer workflow (Option 1)

Clone the git, make your changes, and run make install PREFIX=$HOME/.local

Developer workflow (Option 2)

Clone the git directly into ~/.local/share/gnome-shell/extensions.

Why a separate install and install-tool target?

install just copies the contents of this repository to the install destination folder, e.g. it behaves exactly like the install button on extensions.gnome.org. This is helpful in maintaining a smooth experience when publishing a new version on extensions.gnome.org, as we (the developers) can test the install with make install.

Problems

I need time to implement this 🙈

@fin-ger
Copy link
Member

fin-ger commented Mar 29, 2020

I started working on this

@fin-ger
Copy link
Member

fin-ger commented Mar 31, 2020

Look at #107 for progress

@hackel
Copy link

hackel commented Oct 6, 2020

This tool should not be touching anything outside of /usr/local and /etc. It would be ideal if you had a simple way to just generate the files so they can be expected and then copied into place manually. People should not be encouraged to grant root access to random GNOME Shell extension (no offence).

@fin-ger
Copy link
Member

fin-ger commented Oct 7, 2020

If I get this right, you are missing a make package-tool target which creates a generated tgz for the polkit rules and the cpufreqctl tool. This can then be manually extracted onto the root filesystem.

The core problem of this is that one generated polkit rule only works for a specific installation path of the extension. If the extension got installed system wide or by another user, you need a different polkit rule as when the extension got installed via the gnome extensions website. This is to prevent file collisions on multi user systems where each user installs the extension individually and to prevent privilege escalations from other users on the same system.

However, we could provide a complete extractable tgz for system wide installations only, in the Github releases download section. This tgz contains the shell extension and the corresponding polkit rules for a system wide installation.

Also, we can provide a make package-tool TOOL_SUFFIX=... PREFIX=... copy&paste command as an alternative to the "ask-for-sudo-password-installation". However, I would prefer to have this as an expert option which is not the default, to not make the installation more inaccessible to users.

I would prefer distros to ship this extension. Maybe I'll find the time to create Gentoo and Fedora packages... But I hoped others would get on this as we are mostly lacking time to package for all the distros...

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

No branches or pull requests

4 participants