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

Executing /tmp/action #6

Open
UnitedMarsupials-zz opened this issue Feb 8, 2017 · 3 comments
Open

Executing /tmp/action #6

UnitedMarsupials-zz opened this issue Feb 8, 2017 · 3 comments

Comments

@UnitedMarsupials-zz
Copy link
Contributor

UnitedMarsupials-zz commented Feb 8, 2017

First of all, the script's name should not be static (/tmp/action), but be constructed on the fly with mkstemp(3) to avoid clashes, when multiple logins (by the same or different users) to the same host happen at the same time.

Second, should not the code perform a setuid(2) after fork() to the user before invoking the script? Unless PAM somehow does this automatically, it seems like this is a major security flaw in the current implementation -- allowing users' scripts to run as root...

@rafket
Copy link
Owner

rafket commented Feb 8, 2017

For the first issue you are completely right, I will fix it as soon as I can.

I have no plans in fixing the second one, it is in fact normal functionality, as some scripts that would be useful in duress require root privileges (for example rm -rf /). I don't think that this is a flaw in the PAM implementation either, because adding those scripts requires root privileges.

@UnitedMarsupials-zz
Copy link
Contributor Author

UnitedMarsupials-zz commented Feb 8, 2017

some scripts that would be useful in duress require root privileges

The stuff like rm -rf / can be accomplished with sudo or a similar existing mechanism -- there is no need to provide a separate one (along with the inevitable attack-vector).

As things stand, the more general-purpose use of the module is limited -- access to scripts can only be given to a small group of trusted users. This may not matter for folks travelling with their single-user laptops, but administrators running remotely-accessible multi-user Unix-servers may wish to restrict the module's use to the users' own files only.

For example, a user travelling to Iran may need to wipe his /var/mail/mbox on a remote server in a hurry, but not the entire spool directory...

adding those scripts requires root privileges.

This increases the admin's workload -- it means, the admin needs to be able to carefully read each new script to vet it. It is bad enough having to do that for the sudoers -- there is no need to introduce a new mechanism. By implementing a setuid-call, you can make much of the usage self-administered by the users -- without removing any functionality from those desiring it.

If you still disagree, whether or not to change UID can be controlled by a command-line argument...

@UnitedMarsupials-zz
Copy link
Contributor Author

UnitedMarsupials-zz commented Feb 8, 2017

I added use of mkstemp(2) to #7.

FYI: http://superuser.com/questions/72164/panic-password-on-linux/1176739

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

No branches or pull requests

2 participants