From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4755 invoked by alias); 17 Sep 2015 11:16:20 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 4731 invoked by uid 89); 17 Sep 2015 11:16:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.3 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 17 Sep 2015 11:16:18 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id 1106F461C0; Thu, 17 Sep 2015 11:16:17 +0000 (UTC) Received: from localhost (ovpn-116-24.ams2.redhat.com [10.36.116.24]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t8HBGGOb000333; Thu, 17 Sep 2015 07:16:16 -0400 Date: Thu, 17 Sep 2015 11:31:00 -0000 From: Jonathan Wakely To: Martin Sebor Cc: libstdc++ , gcc-patches Subject: Re: [patch] libstdc++/67173 Fix filesystem::canonical for Solaris 10. Message-ID: <20150917111615.GH2631@redhat.com> References: <20150911142140.GL2631@redhat.com> <55F311D2.8050405@gmail.com> <55F469CF.9010503@gmail.com> <20150916144207.GY2631@redhat.com> <55F9A8A8.3060502@gmail.com> <20150916185844.GD2631@redhat.com> <55F9E75F.10602@gmail.com> <20150916221727.GF2631@redhat.com> <55F9FE71.1060901@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline In-Reply-To: <55F9FE71.1060901@gmail.com> X-Clacks-Overhead: GNU Terry Pratchett User-Agent: Mutt/1.5.23 (2014-03-12) X-SW-Source: 2015-09/txt/msg01272.txt.bz2 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 the value the kernel uses? 20 seems quite low, I was thinking of a much higher number.