From owner-freebsd-hackers@FreeBSD.ORG Tue Jun 2 11:28:36 2009 Return-Path: Delivered-To: freebsd-hackers@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 970301065679 for ; Tue, 2 Jun 2009 11:28:36 +0000 (UTC) (envelope-from des@des.no) Received: from tim.des.no (tim.des.no [194.63.250.121]) by mx1.freebsd.org (Postfix) with ESMTP id 546B78FC23 for ; Tue, 2 Jun 2009 11:28:36 +0000 (UTC) (envelope-from des@des.no) Received: from ds4.des.no (des.no [84.49.246.2]) by smtp.des.no (Postfix) with ESMTP id 5B5156D41E; Tue, 2 Jun 2009 13:28:35 +0200 (CEST) Received: by ds4.des.no (Postfix, from userid 1001) id 3AA16844E2; Tue, 2 Jun 2009 13:28:35 +0200 (CEST) From: =?utf-8?Q?Dag-Erling_Sm=C3=B8rgrav?= To: rea-fbsd@codelabs.ru References: <23727599.post@talk.nabble.com> <86prdvipwe.fsf@ds4.des.no> <20090527233110.E4243@delplex.bde.org> <86r5yaijef.fsf@ds4.des.no> <20090529210855.V1643@besplex.bde.org> <86vdnju9z1.fsf@ds4.des.no> <86r5y7u9r3.fsf@ds4.des.no> Date: Tue, 02 Jun 2009 13:28:34 +0200 In-Reply-To: (Eygene Ryabinkin's message of "Tue, 2 Jun 2009 13:51:59 +0400") Message-ID: <86skiiri1p.fsf@ds4.des.no> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.0.92 (berkeley-unix) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Cc: freebsd-hackers@FreeBSD.org, Jakub Lach , Bruce Evans Subject: Re: FYI Lighttpd 1.4.23 /kernel (trailing '/' on regular file symlink) vulnerability X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 02 Jun 2009 11:28:37 -0000 Eygene Ryabinkin writes: > For the current code state, check "*cp =3D=3D '\0'" seems to be redundant: > [...] By the way, comment before the test "if (rdonly)' seems to be > slightly misleading [...] OK, I see that. I've removed the check and rewritten the comment. What about this "temporary assert"? /* * This is a temporary assert to make sure I know what the * behavior here was. */ KASSERT((cnp->cn_flags & (WANTPARENT|LOCKPARENT)) !=3D 0, ("lookup: Unhandled case.")); It was added by jeff@ four years ago, with the following log message: - Add a few asserts for some unusual conditions that I do not believe can happen. These will later go away and turn into implementations for these conditions. Any reason not to remove it? > Seems like here we can also check for the trailing slash to be set on > any previous invocation (or the current one), since we're following > symlinks in the case of directories, so we should follow them to the > end. I'm not sure I understand... > Perhaps 'XXX for direnter()' should be changed to something like > 'strip trailing slashes in cnp->cn_nameptr'. I'll just remove it, since the previous comment clearly explains what is going on. > By the way, comment just after 'nextname' label is misleading: we can be > there with symbolic link, but it won't be followed. So "Not a symbolic > link" can be changed to something like "Not a symbolic link that will > be followed". /* * Not a symbolic link that we will follow. Continue with the * next component if there is any; otherwise, we're done. */ DES --=20 Dag-Erling Sm=C3=B8rgrav - des@des.no