[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: cp, ln, mv, install: check for vulnerable target directories
From: |
Paul Eggert |
Subject: |
Re: cp, ln, mv, install: check for vulnerable target directories |
Date: |
Wed, 20 Sep 2017 00:06:14 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 |
Pádraig Brady wrote:
It would be better to have some discussion about such patches before landing.
Feel free to revert the whole thing if you'd prefer; I won't mind. (I would like
the security hole to be smaller, though....(
I see it discounts sticky dirs, which is a bit surprising since they're
generally the most vulnerable since they're configured to be shared?
It's the other way round. If you do 'cp foo /tmp/d' and /tmp is world-writable
and sticky and you own d (without following d if it is a symlink), then no other
user can attack you by turning d into a symlink to a victim directory. So
bleeding-edge cp doesn't fail in that case. It would fail if /tmp were
world-writable and non-sticky, though, as that can be hijacked. In other words,
sticky directories are safer than non-sticky ones.
Note the sticky dir case is already handled at the Linux kernel level
with /proc/sys/fs/protected_symlinks, as that's the main attack vector.
I wasn't aware of that feature. Yes, it closes the most-obvious big hole of
symlink attacks via /tmp. However, it doesn't close the hole of a symlink in a
non-sticky world-writable directory, which is odd as that is a wider hole than a
sticky world-writable directory like /tmp. And I'm puzzled why it follows a
symlink merely because the symlink and its parent directory have the same owner,
as this lets a directory's owner to hijack attempts by other users to copy into
the directory. In short, the protected_symlinks feature doesn't seem to be
designed to thwart symlink attacks in general, just ones directly via /tmp.
Also, /proc/sys/fs/protected_symlinks is just a Linux feature, right? It's nicer
to have something that also works in other POSIXish systems.
I feel this is protecting a less problematic case and will probably
break embedded system scripts for example which generally don't
care about permissions but run stuff as non root.
I'm not sure what running as non root has to do with it. The coreutils heuristic
pretty much does the same thing for root as for non-root users.
I take your point about embedded systems. Though don't they generally have
directories that are mode 755? If so, it shouldn't be much of a problem with
this patch. If not, perhaps we should look at the permissions of /, and bypass
all this checking if / is world-writable.
this doesn't protect
the case where root is writing to a root owned but a+w dir,
That's not the problem. The problem is when the destination's parent is a+w, not
when the destination itself is an a+w dir. And the newly-installed patch defends
against this:
# ls -ld /tmp/d /tmp/d/passwd
drwxrwxrwx 2 root root 4096 Sep 19 23:31 /tmp/d
lrwxrwxrwx 2 whoopsie whoopsie 7 Sep 19 23:31 /tmp/d/passwd -> /etc
# src/cp passwd /tmp/d/passwd
src/cp: vulnerable target directory '/tmp/d/passwd' (append '/' to use anyway)
whereas coreutils 8.28 cp charges ahead and overwrites /etc/passwd here.
> it seems strange to match the dir owner with the writer.
It's because we're worried about the destination's parent, not the destination.
And the troublesome case occurs when a non-trusted user can write to the
destination's parent. We trust ourselves and root, so if no user other than
ourselves or root can write to the parent, then it's safe. I.e., it's not safe
if the parent is owned by somebody other than ourself or root, because that
somebody-else could hijack us.
In the kernel protected_symlinks case the owner match gating is
on the symlink itself which makes sense.
Coreutils uses lstat on the destination, which means it's gating on the symlink
itself too.
I'd be more inclined
to have another kernel value for /proc/sys/fs/protected_symlinks
that also provided the protection to non sticky dirs?
That would help. It'd also help if a kernel value could close the
symlink-has-same-owner-as-parent-dir hole. (I'm curious as to why
protected_symlinks seems to be missing the point here....)
A less invasive change might be to enable this only with -i
which Fedora at least enables by default for the root user.
Only the root user? I'm also worried about non-root users being hijacked.
Also there were many syntax-check failures in this patch.
Yes, I often forget to check those. I will try to do better....
- cp, ln, mv, install: check for vulnerable target directories, Paul Eggert, 2017/09/19
- Re: cp, ln, mv, install: check for vulnerable target directories, Pádraig Brady, 2017/09/20
- Re: cp, ln, mv, install: check for vulnerable target directories, Kaz Kylheku (Coreutils), 2017/09/20
- Re: cp, ln, mv, install: check for vulnerable target directories, Paul Eggert, 2017/09/20
- Re: cp, ln, mv, install: check for vulnerable target directories, Kaz Kylheku (Coreutils), 2017/09/21
- Re: cp, ln, mv, install: check for vulnerable target directories, Kaz Kylheku (Coreutils), 2017/09/21
- Re: cp, ln, mv, install: check for vulnerable target directories, Kaz Kylheku (Coreutils), 2017/09/21
- Re: cp, ln, mv, install: check for vulnerable target directories, Paul Eggert, 2017/09/21
- Re: cp, ln, mv, install: check for vulnerable target directories, Paul Eggert, 2017/09/25
Re: cp, ln, mv, install: check for vulnerable target directories, Kaz Kylheku (Coreutils), 2017/09/20
- Prev by Date:
Re: cp, ln, mv, install: check for vulnerable target directories
- Next by Date:
Re: cp, ln, mv, install: check for vulnerable target directories
- Previous by thread:
Re: cp, ln, mv, install: check for vulnerable target directories
- Next by thread:
Re: cp, ln, mv, install: check for vulnerable target directories
- Index(es):