Date: Tue, 02 Jun 2009 13:28:34 +0200 From: =?utf-8?Q?Dag-Erling_Sm=C3=B8rgrav?= <des@des.no> To: rea-fbsd@codelabs.ru Cc: freebsd-hackers@FreeBSD.org, Jakub Lach <jakub_lach@mailplus.pl>, Bruce Evans <bde@zeta.org.au> Subject: Re: FYI Lighttpd 1.4.23 /kernel (trailing '/' on regular file symlink) vulnerability Message-ID: <86skiiri1p.fsf@ds4.des.no> In-Reply-To: <qlE9VS4n%2BoBC3U4EHMxYsjC7RHI@XX1fo6zQUfC4h0jjRC6IBz3oNH4> (Eygene Ryabinkin's message of "Tue, 2 Jun 2009 13:51:59 %2B0400") 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> <qlE9VS4n%2BoBC3U4EHMxYsjC7RHI@XX1fo6zQUfC4h0jjRC6IBz3oNH4>
next in thread | previous in thread | raw e-mail | index | archive | help
Eygene Ryabinkin <rea-fbsd@codelabs.ru> 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?86skiiri1p.fsf>