Re: cp, ln, mv, install: check for vulnerable target directories

[Pádraig Brady]

On 19/09/17 00:25, Paul Eggert wrote:
> For years cp and friends have been subject to a symlink attack, in that 
> seemingly-ordinary commands like 'cp a b' can overwrite arbitrary directories 
> that the user has access to, if b's parent directory is world-writable and is 
> not sticky and is manipulated by a malicious user. To help ameliorate this 
> problem, I've installed the attached patch into coreutils. Although it does 
> not 
> detect every instance of this problem, the goal is that its heuristic should 
> avoid false positives while catching and reporting and refusing to act on 
> most 
> instances.
> 
> As this is not an upward-compatible change, I installed this patch right 
> after a 
> coreutils release, in the hope that we will have time to shake out any 
> problems 
> with it (including reverting it entirely :-) before the next release.
> 

It would be better to have some discussion about such patches before landing.

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?
I suppose that would disallow copying to /tmp on most systems which
would be a bit drastic, but this is that drastic in the less problematic
case of non sticky world writeable dirs.

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 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. BTW this doesn't protect
the case where root is writing to a root owned but a+w dir, so
it seems strange to match the dir owner with the writer.
In the kernel protected_symlinks case the owner match gating is
on the symlink itself which makes sense.  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?

A less invasive change might be to enable this only with -i
which Fedora at least enables by default for the root user.

Also there were many syntax-check failures in this patch.
I've fixed them up in a separate patch
(isolated from yet more syntax-check failures)
so we can revert these in reverse order if needed.

Pádraig