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>