htop: binary should be installed SGID procmod, not SUID root

[mklein-de (Michael Klein)]

when installing as root, htop is installed SUID root, which allows any user to kill arbitrary processes.

instead, htop should be installed SGID procmod.

The attached patch adds a "procmod" variant (set as default) which does exactly this. -procmod should only be used when installing without root privileges.

[mklein-de (Michael Klein)]

Portfile patch

[larryv (Lawrence Velázquez)]

Ergh. That’s no good. Elevating priority due to security implications.

FYI, "Cc" takes full email addresses, not MacPorts handles.

[neverpanic (Clemens Lang)]

I agree, installing SUID root is not a good thing in this case. I don't think I was able to kill random processes without running htop as sudo htop, though. I wonder whether my system is different there, but can't check at the moment.

I also wonder whether there is a way to automatically determine when we're in a non-root installation and change the default variants accordingly.

[nonstop.server@…]

Cc Me!

[neverpanic (Clemens Lang)]

Fixed by patching Makefile.am (we need to run autoconf anyway) in r102085.

[neverpanic (Clemens Lang)]

Using this change, htop can no longer display the command line of processes belonging to other users (it will only display the program basename). Is there a way to fix this?

[frozencemetery (Robbie Harwood)]

Speaking of non-root installations, installing htop now fails if the user is not a member of group procmod (i.e., if the install user is not root). This is a regression, as installing htop as the non-root user worked previously.

[frozencemetery (Robbie Harwood)]

Cc Me!

[neverpanic (Clemens Lang)]

The regression should be fixed in r102094. Please run selfupdate and try again.

[frozencemetery (Robbie Harwood)]

Replying to cal@…:

The regression should be fixed in r102094. Please run selfupdate and try again.

Looks good, thanks!

[neverpanic (Clemens Lang)]

Please do not close this issue until we have discussed if (and how) full functionality of htop can be restored without SUID root.

[raimue (Rainer Müller)]

Replying to cal@…:

Please do not close this issue until we have discussed if (and how) full functionality of htop can be restored without SUID root.

I doubt this can be restored. For example, /bin/ps is also configured as SUID root. If you lower it's permissions it only shows the base name in parentheses for processes of other users. As far as I checked, both are using task_for_pid() and task_info(), which are restricted to root or signed applications (via authorization policies controlled by taskgated(8) using rules from /etc/authorization).

According to man page taskgated(8), legacy versions of OS X granted permissions for procmod and procview. I am not even sure whether the group procmod does anything useful at the moment. I did not notice a change in the behavior of htop whether the permissions are root:procmod 2755 or root:admin 0755.

[raimue (Rainer Müller)]

Cc Me!

[mklein-de (Michael Klein)]

Replying to raimue@…:

Replying to cal@…:

Please do not close this issue until we have discussed if (and how) full functionality of htop can be restored without SUID root.

I doubt this can be restored. For example, /bin/ps is also configured as SUID root.

So just leave it SUID root then and add additional checks in the code? I can think of four places that need an additional check:

• killing processes (obviously)
• raising/lowering priority
• the call to lsof(8)
• the call to strace (doesn't exist in OS X, check still required)

I'm attaching a patch to close these holes, but I'm not sure if there are more :-/

According to man page taskgated(8), legacy versions of OS X granted permissions for procmod and procview. I am not even sure whether the group procmod does anything useful at the moment. I did not notice a change in the behavior of htop whether the permissions are root:procmod 2755 or root:admin 0755.

I can't speak for recent versions, but on 10.5, memory information is only shown for the htop process itself in the second case.

[mklein-de (Michael Klein)]

add additional checks when running SUID root

[neverpanic (Clemens Lang)]

I think that should be it. Applied in r102162. Thank you for both the report and the patch.