Quotes of the week

Posted Jun 4, 2010 2:44 UTC (Fri) by viro (subscriber, #7872)
In reply to: Quotes of the week by spender
Parent article: Quotes of the week

mkdir /tmp/c
ln -s `readlink /tmp/a`/b /tmp/c/b
mv /tmp/a /tmp/d
mv /tmp/c /tmp/a

will be allowed for attacker and result will have application that tries to open /tmp/a/b hitting exactly what it would hit before that sequence, without triggering the checks. Again, I'm *NOT* saying that we need to try saving such applications from themselves; just that patch as posted doesn't in fact protect them. All it really gives is protection for the final component, assuming that everything prior to that walks through secure places. Which it does, in all practically interesting cases.

BTW, "security theatre" was applied to attempts to handwave the patch through, without bothering with the analysis of what it really changed. "It mitigates the ugly holes we want to have covered, and it's surely better to be more thorough". Note that analysis hadn't been given _anywhere_ upthread on l-k ;-/

Anyway, I'm out of that discussion. Flogging the dead horses is boring and this one is a smear on the ground by now...

to post comments

Quotes of the week

Posted Jun 4, 2010 12:28 UTC (Fri) by spender (guest, #23067) [Link] (1 responses)

Indeed that works, I mentioned that it only prevented the case where /tmp/a was a symlink. In your case it's a regular directory. If we assume a blind echo "blah" > /tmp/a/b then indeed the check is bypassed. But /tmp/a/b in the symlinked 'a' case isn't always equivalent to the case where 'a' is a normal directory.

Some simple examples that I don't think are too far-fetched:
If the name of 'b' can't be determined a priori or the directory can't be filled exhaustively with all possible names for 'b', then the symlinked 'a' is required.
A program using /tmp/a/b would have some logic for creating the directory if it doesn't exist and additional logic for when it does exist.
If it does exist, the app could do a number of things, two being:
* check the ownership of the directory (using stat())
* enter the directory and remove all the files inside it
In both of these cases the non-symlinked 'a' doesn't allow for an attack. With the symlinked 'a', the ownership of the directory (assuming we're targeting root) would be root, with the normal directory it'd have ownership of the attacker.
If the application goes in and removes all the files in the directory, in the symlinked 'a' case, it removes files in the directory the attacker is targeting. In the non-symlinked case it simply removes the attacker's symlinks.

I imagine we'll have to agree to disagree on the merits of these differences.

-Brad

Quotes of the week

Posted Jun 4, 2010 20:27 UTC (Fri) by vonbrand (subscriber, #4458) [Link]

So it gets to be "Let's save programmers form making dumb mistakes by asking them _try_ the dumb mistake blindly and then do it right if the first try fails"... I don't see the logic behind this.

I'm with Al Viro here: The only solution that fixes all troublesome cases and doesn't litter the landscape with all sort of weird special cases is to forbid symlinks completely. That one hurts way too much. The sane solution is just to disallow hard links to outside {/var,}/tmp... i.e., a separate partition (even tmpfs!) or just mount it over itself. Problem gone, no strange special cases, any sane use for the above behavior still works, no kernel changes.