Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#37766 closed defect (fixed)

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

Reported by: mklein-de (Michael Klein) Owned by: neverpanic (Clemens Lang)
Priority: High Milestone:
Component: ports Version: 2.1.2
Keywords: haspatch Cc: nonstop.server@…, frozencemetery (Robbie Harwood), raimue (Rainer Müller)
Port: htop

Description

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.

Attachments (2)

htop-Portfile.diff (678 bytes) - added by mklein-de (Michael Klein) 8 years ago.
Portfile patch
patch-suid-privchecks.diff (2.3 KB) - added by mklein-de (Michael Klein) 8 years ago.
add additional checks when running SUID root

Download all attachments as: .zip

Change History (17)

Changed 8 years ago by mklein-de (Michael Klein)

Attachment: htop-Portfile.diff added

Portfile patch

comment:1 Changed 8 years ago by larryv (Lawrence Velázquez)

Cc: cal removed
Owner: changed from macports-tickets@… to cal@…
Priority: NormalHigh

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

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

comment:2 Changed 8 years ago by neverpanic (Clemens Lang)

Status: newassigned

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.

comment:3 Changed 8 years ago by nonstop.server@…

Cc: nonstop.server@… added

Cc Me!

comment:4 Changed 8 years ago by neverpanic (Clemens Lang)

Resolution: fixed
Status: assignedclosed

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

comment:5 Changed 8 years ago by neverpanic (Clemens Lang)

Resolution: fixed
Status: closedreopened

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?

comment:6 Changed 8 years ago by 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.

comment:7 Changed 8 years ago by frozencemetery (Robbie Harwood)

Cc: rharwood@… added

Cc Me!

comment:8 Changed 8 years ago by neverpanic (Clemens Lang)

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

comment:9 in reply to:  8 Changed 8 years ago by frozencemetery (Robbie Harwood)

Replying to cal@…:

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

Looks good, thanks!

comment:10 Changed 8 years ago by jmroot (Joshua Root)

Resolution: fixed
Status: reopenedclosed

comment:11 Changed 8 years ago by neverpanic (Clemens Lang)

Resolution: fixed
Status: closedreopened

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

comment:12 in reply to:  11 ; Changed 8 years ago by 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.

comment:13 Changed 8 years ago by raimue (Rainer Müller)

Cc: raimue@… added

Cc Me!

comment:14 in reply to:  12 Changed 8 years ago by 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.

Changed 8 years ago by mklein-de (Michael Klein)

Attachment: patch-suid-privchecks.diff added

add additional checks when running SUID root

comment:15 Changed 8 years ago by neverpanic (Clemens Lang)

Resolution: fixed
Status: reopenedclosed

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

Last edited 8 years ago by neverpanic (Clemens Lang) (previous) (diff)
Note: See TracTickets for help on using tickets.