Re: [patch] libstdc++/67173 Fix filesystem::canonical for Solaris 10.

[Jonathan Wakely <jwakely at redhat dot com>]

On 17/09/15 09:37 -0600, Martin Sebor wrote:
> On 09/17/2015 05:16 AM, Jonathan Wakely wrote:
> > On 16/09/15 17:42 -0600, Martin Sebor wrote:
> > > I see now the first exists test will detect symlink loops in
> > > the original path. But I'm not convinced there isn't a corner
> > > case that's subject to a TOCTOU race condition between the first
> > > exists test and the while loop during which a symlink loop can
> > > be introduced.
> > >
> > > Suppose we call the function with /foo/bar as an argument and
> > > the path exists and contains no symlinks. result is / and cmpts
> > > is set to { foo, bar }. Just as the loop is entered, /foo/bar
> > > is replaced with a symlink containing /foo/bar. The loop then
> > > proceeds like so:
> > >
> > > 1. The first iteration removes foo from cmpts and sets result
> > > to /foo. cmpts is { bar }.
> > >
> > > 2. The second iteration removes bar from cmpts, sets result to
> > > /foo/bar, determines it's a symlink, reads its contents, sees
> > > it's an absolute pathname and replaces result with /. It then
> > > inserts the symlink's components { foo, bar } into cmpts. cmpts
> > > becomes { foo, bar }. exists(result) succeeds.
> > >
> > > 3. The next iteration of the loop has the same initial state
> > > as the first.
> > >
> > > But I could have very easily missed something that takes care
> > > of this corner case. If I did, sorry for the false alarm!
> >
> > No, you're right. The TS says such filesystem races are undefined:
> > http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4099.html#fs.race.behavior
> >
> > but it would be nice to fail gracefully rather than DOS the
> > application.
> >
> > The simplest approach would be to increment a counter every time we
> > follow a symlink, and if it reaches some limit decide something is
> > wrong and fail with ELOOP.
> >
> > I don't see how anything else can be 100% bulletproof, because a truly
> > evil attacker could just keep altering the contents of symlinks so we
> > keep ping-ponging between two or more paths. If we keep track of paths
> > we've seen before the attacker could just keep changing the contents
> > to a unique path each time, that initially exists as a file, but by
> > the time we get to is_symlink() its become a symlink to a new path.
> >
> > So if we use a counter, what's a sane maximum? Is MAXSYMLINKS in
> > <sys/param.h> the value the kernel uses? 20 seems quite low, I was
> > thinking of a much higher number.
>
> Yes, it is a corner case, and it's not really avoidable in the case
> of hard links. For symlinks, POSIX defines the SYMLOOP_MAX constant
> as the maximum, with the _SC_SYMLOOP_MAX and _PC_SYMLOOP_MAX
> sysconf and pathconf variables. Otherwise 40 seems reasonable.
>
> With this, I'll let you get back to work -- I think we've beat this
> function to death ;)

Here's what I committed. Similar to the last patch, but using the new
is_dot and is_dotdot helpers.

Attachment: patch-fs-3.txt
Description: Text document