FYI Lighttpd 1.4.23 /kernel (trailing '/' on regular file symlink) vulnerability

[Dag-Erling Smørgrav]

Eygene Ryabinkin <rea-fbsd at codelabs.ru> writes:
> For the current code state, check "*cp == '\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)) != 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
-- 
Dag-Erling Smørgrav - des at des.no