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

Remove use of Environment Python #904

Open
wants to merge 5 commits into
base: beta
Choose a base branch
from
Open

Conversation

dthelegend
Copy link

@dthelegend dthelegend commented Apr 10, 2024

Hi,

I was running this on Arch Kernel 6.8.2 and I recently moved from the howdy AUR package to the howdy-beta-git package.

I use Pyenv to manage multiple python versions and howdy seems to always want to use the environment python, which I believe is incorrect behavior, especially from a security standpoint wherein (this example is admittedly quite stupid, but I hope it tracks) a malicious program could launch a subprocess with sudo that uses howdy and a modified environment to launch an executable of the attackers choice from the PAM context as long as it is named python.

This patch removes the use of the "!/usr/bin/env python3" and replaces it with "!/usr/bin/python3" and also changes the python3 executable to "/usr/bin/python3" in "compare.py".

I have patched and installed this on my local system and it seems to work fine.

I hope this helps!

Copy link
Collaborator

@musikid musikid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're absolutely right, although it would be better to allow the packages' maintainers to specify the path because it isn't always guaranteed to be in /usr/bin.

howdy/src/compare.py Outdated Show resolved Hide resolved
howdy/src/cli.py Outdated Show resolved Hide resolved
howdy-gtk/src/init.py Outdated Show resolved Hide resolved
howdy-gtk/bin/howdy-gtk.in Outdated Show resolved Hide resolved
howdy/src/pam/main.cc Outdated Show resolved Hide resolved
howdy/src/bin/howdy.in Outdated Show resolved Hide resolved
@dthelegend
Copy link
Author

Apologies I have not had a chance to review and test your changes, my laptop died last Monday and I'm still out of a laptop. I'd say that just on a skim your changes look good 👍🏾

Thanks @musikid

Co-authored-by: Sayafdine Said <sayafdine.said+github@protonmail.com>
@musikid musikid linked an issue May 6, 2024 that may be closed by this pull request
@dthelegend
Copy link
Author

dthelegend commented May 20, 2024

Hi, my laptop is still dead and ASUS want to charge me £1200 so I'll be out of a laptop a bit longer. I'm making this PR from a backup PC, but I don't have any cameras, so whether this works is uncertain, but it compiles and I used grep to check the strings have been correctly replaced.

I HIGHLY ADVISE SOMEONE WHO CAN TEST IT PLEASE TEST IT. Otherwise I think this is ready to merge in.

EDIT: Also I use Arch (btw), so I have tested the makepkg and that works fine, but I haven't tested the Debian or Fedora package

@dthelegend dthelegend requested a review from musikid May 20, 2024 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to use Howdy with software managing multiple Python versions
2 participants