Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 81097 - portage should warn about suid/sgid binaries with remaining links.
Summary: portage should warn about suid/sgid binaries with remaining links.
Alias: None
Product: Portage Development
Classification: Unclassified
Component: Core (show other bugs)
Hardware: All All
: High normal (vote)
Assignee: Portage team
Keywords: InVCS
Depends on: 81165
Blocks: 193766 181949
  Show dependency tree
Reported: 2005-02-07 04:51 UTC by Tavis Ormandy (RETIRED)
Modified: 2007-09-25 16:34 UTC (History)
2 users (show)

See Also:
Package list:
Runtime testing required: ---

portage nlinks patch (portage-nlinks.diff,1.35 KB, patch)
2005-02-07 12:41 UTC, Tavis Ormandy (RETIRED)
Details | Diff
portage nlinks patch v1.01 (portage-nlinks.diff,3.00 KB, patch)
2005-02-07 16:06 UTC, Tavis Ormandy (RETIRED)
Details | Diff
portage suid/sgid nlinks checking (portage-nlinks.diff,3.04 KB, patch)
2005-02-10 02:32 UTC, Tavis Ormandy (RETIRED)
Details | Diff
use inode reference counts to prevent false positives (security_check.patch,3.54 KB, patch)
2007-06-10 23:07 UTC, Zac Medico
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tavis Ormandy (RETIRED) gentoo-dev 2005-02-07 04:51:02 UTC
Say you have a package, foobar-1.2, that contains the suid root executable "foo".

$ ls -l /usr/bin/foo
-rws--x--x  1 root root 833890 Feb  7 12:02 /usr/bin/foo*

foobar-1.2 is found to have a security issue and allows local user to escalate privileges, gentoo fixes the problem, issues an advisory and portage replaces the vulnerable binary with the fixed one...problem solved.

however, just unlinking the binary won't remove all links to it, a malicious user might have created a hardlink to it in /tmp, for example:

$ ln /usr/bin/foo /tmp/vulnerable_foo
$ ls -l vulnerable
-rws--x--x  2 root root 833890 Feb  7 12:02 vulnerable_foo*

unlinking and replacing /usr/bin/foo is not sufficient to stop users from exploiting this vulnerable binary. In theory a user who is set on getting root could just create hardlinks to all the suid/sgid applications on the fileystem in his homedir, waits for a security advisory to be issues, then even if the admin has applied the update the user can still exploit it and take his time about developing an exploit or whatever.

Okay, the answer is always mount /home /var /tmp etc with nosuid, but 99% of users don't follow this advice and don't understand that they might still have vulnerable suid binaries on their system...

It would be very simple to issue an ewarn style warning if the number of links to a suid/sgid binary is > 1 (as reported by stat()), and could warn admins about users abusing hardlinks.

just something like:

* WARNING: suid binary /usr/bin/gpg has st_nlink > 1
Comment 1 Thierry Carrez (RETIRED) gentoo-dev 2005-02-07 05:12:05 UTC
99% of the users won't know what to do with a cryptic warning like this.

Maybe automate removal of all hardlinks to SUID binaries (if possible) during package removal, with a notice ? And make the non-autoremoval of hardlinks (and the warning) a "nosuidhardlinkremove" feature for those in the know (who use hardlinks to SUID binaries and want to keep them) ?
Comment 2 Ed Grimm 2005-02-07 07:26:14 UTC
In response to comment #1,
Agreed.  In no event should portage remove the instance it knows about and not remove all the others on the system.

Note that portage should track instances where a package installs with hardlinks.  Not sure if it does or not.  However, it would make it a lot simpler if it can see the package installs with three hardlinks to this file, there are three hardlinks to the file, and all the ones it knows about are present, so it doesn't need to go on a filesystem-wide trawl looking for the other links.
Comment 3 Tavis Ormandy (RETIRED) gentoo-dev 2005-02-07 12:41:19 UTC
Created attachment 50654 [details, diff]
portage nlinks patch

How about this, only a few lines of code, and gives an informational message.

I'd be happy to write a paragraph for the gentoo security guide about hardlink
abuse and what it means in easy to understand language.
Comment 4 Marius Mauch (RETIRED) gentoo-dev 2005-02-07 13:03:10 UTC
Let us know when the security guide has the relevant section so we don't apply the patch before and get completely confused users.
Comment 5 Tavis Ormandy (RETIRED) gentoo-dev 2005-02-07 13:43:19 UTC
Okay, see patch attached to bug 81165 for an update to docs :)
Comment 6 solar (RETIRED) gentoo-dev 2005-02-07 14:05:03 UTC
Good idea Tavis :)
Comment 7 Tavis Ormandy (RETIRED) gentoo-dev 2005-02-07 16:06:08 UTC
Created attachment 50680 [details, diff]
portage nlinks patch v1.01

also check on merge, no additional overhead from another stat() required, I
just reorganized it to use the exisiting one.
Comment 8 Tavis Ormandy (RETIRED) gentoo-dev 2005-02-10 02:32:39 UTC
Created attachment 50901 [details, diff]
portage suid/sgid nlinks checking

Oops, i didnt realise the existing stat could fail, only test if it's been set.
Comment 9 Tavis Ormandy (RETIRED) gentoo-dev 2005-02-14 09:47:15 UTC
Security guide has been updatde:
Comment 10 Ed Grimm 2005-03-16 23:58:22 UTC
It's really nice that the security guide has been updated, but I don't think that makes a lot of
difference - most people don't know what all the files an emerge is going to delete before it does
them.  So long as /tmp, /usr/tmp, /var/tmp, or /home is on the same partition as /, and users are
not running hardened Gentoo, I'm not certain they can reasonably prevent linking.  [note: the
default /etc/fstab, which Gentoo insanely tries to push every baselayout update, does not contain
a separate /home, /tmp, /var, /var/tmp, or /usr/tmp partition.]  Sure, they can block access to ln,
but they also need to block access to python, perl, and a host of other utilities to pull it off.  It
greatly limits the viability of the system, because so many things give people intended or
unintended access to link.

As far as the viability of running hardened Gentoo - I've run into dozens of problems, most of
which I've tracked down to the fact that I am running hardened Gentoo.  For example, under the
full hardened setup, python won't pass the tests it needs to in order to install.  (bug 67970)  Most
of these, I've been able to kludge around myself; but I doubt the average Gentoo user could pull it
off.  I would suggest anyone who thinks otherwise does not realize how much superior their Linux
skills are than the average user.

I'm not a python programmer, and I haven't looked at how attachment 50901 [details, diff] fits in with the rest
of the code, but it looks like it deletes the suid files before verifying it has all the links, especially
based upon the last error message.  As I mentioned before, that's a "game over" situation; this is
especially true as the device and inode numbers are not reported (although these would be useless
to the average user.)
Comment 11 Tavis Ormandy (RETIRED) gentoo-dev 2005-03-17 02:06:50 UTC
Ed: "I'm not certain they can reasonably prevent linking".

Is that a problem? linking is a safe and useful process, you cant get any additional access rights. The problem is users can use it to keep old copies of suid binaries around allowing them time to exploit any discovered problems.

"As far as the viability of running hardened Gentoo..."

This bug is nothing to do with Hardened Gentoo, presumably there are already linking restrictions in place making this bug moot for hardened gentoo users.

"As I mentioned before, that's a "game over" situation;"

Well, I think you're overly concerned with links in general. The patch doesnt change any of the existing portage behaviour, it just prints a warning if any suid/sgid files have multiple links. But you're right, it could print an inode number to allow it to be tracked down, that would be a minor modification, not really justification for rejecting it.

I don't really understand your argument that this is too confusing for "average users", this is a realtively complex issue that doesnt correlate with the standard "files and directories" abstraction that some users are comfortable with, I wrote a passage explaining it for the security guide and the warning message includes a link. The chances of any users who are not systems administrators ever seeing this message is slim to none.

What do you suggest? just forget the idea because mom+pop wont be able to understand it? I think the chances of getting a patch checked in that traverses the filesystem to locate matching inode numbers is slim to none, as cool as that would be. A find command that does that can be easily added to the security guide if your concern is that users wont be able to construct the appropriate command line.

If you think this is not "better than nothing", then I would have to disagree.
Comment 12 Tavis Ormandy (RETIRED) gentoo-dev 2005-03-17 05:22:50 UTC
Ed: I've re-read your other comments, I'm not sure you understand the issue. Let me just spell it out that this is a _very_ specific issue, there are no security issues with allowing users to create hard links, they are not magical gateways to uid 0.

This bug is about this one specific issue: users can use hardlinks to circumvent portage security updates of suid/sgid binaries.

"Note that portage should track instances where a package installs with hardlinks.",

of course, it already does this (portage makes no distinction between files with multiple links and multiple files).

"However, it would make it a lot simpler if it can see the package installs with three hardlinks to this file"

Make what a lot simpler? Portage already manages every installed file, tracking inode numbers and number of links seems to add unnescessary complexity to something that doesnt require it.

I just want to make sure you understand, there is no security issue with multiple links, users are free to create them, just as they are free to make copies of system binaries if they want. This is just about the method of circumventing portage security updates.
Comment 13 Ed Grimm 2005-03-17 21:59:03 UTC
I thought my comments were fairly clear, if you'd read up on the whole thread and related material:

The current patch being proposed appears to warn about the situation of links to suid/sgid binary
files which are not listed in the package contents.  However, if the operation being performed is a
clean, the files that the package does track, and reports, are removed in spite of the situation.  If
the operation being performed is an upgrade, the files in question are overwritten anyway.  At no
point in time does this report the inode numbers.  My understanding of what the code is
potentially in error, as I haven't tracked everything down, and I'm not really a python programmer.
However, if my assessment of what the code does as described in this paragraph is accurate, what
this patch tells the user is effectively, "Unless you're the one that linked to this executable, or you
know how to use find fairly well and have plenty of time to waste on the matter, if the suid/sgid
binary in question has a security hole, you've been hax0red."

The security guide update contains the text "If your users have access to a partition that isn't
mounted with nosuid or noexec (for example, if /tmp, /home, or  /var/tmp are not seperate
partitions) you should take care to  ensure your users don't create hardlinks to SUID or SGID
binaries, so that  after Portage updates they still have access to the old versions."

How is the average Gentoo admin going to do that, while not running hardened Gentoo?

My answer is that they are not going to do it.  They can't.  Their OS does not have utilities for
distinguishing between links to regular files and suid/sgid binaries, and they don't have tools for
tracking down illegitimate suid/sgid binaries before running emerge.  Their only remaining
options are to prevent users from having write access to any partitions with suid/sgid binaries (recommended in the security guide), and to prevent hard links entirely, and I explained what is
wrong with both of those ideas.

If the patch reports the inode, they can recover, so long as they see the message.  Given emerge's
wonderful track record in communicating these things to users, I don't think that's sufficient.  If
instead we abort the operation - do not do any of the clean/merge activity - emerge also aborts,
this is the last message on the screen, the user will see it, and will be able to fix the problem.
Comment 14 Tavis Ormandy (RETIRED) gentoo-dev 2005-03-18 02:41:15 UTC
Ed: you're making a lot of noise about a little issue, this is only a one or two line modification to the patch. As I said above, I agree on this point, I'll add it when I get a chance.

"How is the average Gentoo admin going to do that, while not running hardened Gentoo?"

By being vigilant?

1. admin runs security update, portage warns of remaining links
2. admin tracks down the remaining link with find xxxx -inum x
3. admin removes it.

There's no threat from creating links to suid binaries that dont have security problems.
Comment 15 solar (RETIRED) gentoo-dev 2007-06-05 17:33:22 UTC
This is what got merged into trunk.
if statobj.st_mode & (stat.S_ISUID | stat.S_ISGID):
        # Always blind chmod 0 before unlinking to avoid race conditions.
        os.chmod(obj, 0000)
        if statobj.st_nlink > 1:
                writemsg("setXid: "+str(statobj.st_nlink-1)+ \
                        " hardlinks to '%s'\n" % obj)
Comment 16 Zac Medico gentoo-dev 2007-06-05 20:37:55 UTC
(In reply to comment #15)
> This is what got merged into trunk.

It's been merged into the 2.1.2 branch and released in portage-
Comment 17 Zac Medico gentoo-dev 2007-06-08 22:33:24 UTC
I'd like to integrate Taviso's entire patch.  I'm wondering if we should make the merge/unmerge die when hardlinks to suid/sgid files are detected.  Otherwise, it seems like the message might go unnoticed.  In order to prevent false positives, the code can check if the package in question owns the hardlink and only die if the hardlink can't be accounted for.
Comment 18 Zac Medico gentoo-dev 2007-06-10 23:07:22 UTC
Created attachment 121712 [details, diff]
use inode reference counts to prevent false positives

This new version is in svn r6795 (patch applies against portage-  False positives are prevented by doing reference counts for each inode having suid/sgid bits and multiple hardlinks.  The security check is done prior to each merge or unmerge phase and it will cause the phase to abort if a problem is found (so that the user can investigate before any files are removed).
Comment 19 Zac Medico gentoo-dev 2007-06-15 03:24:04 UTC
This has been released in 2.1.3_rc1.