Date: Tue, 26 May 2009 22:13:21 +0200 From: =?utf-8?Q?Dag-Erling_Sm=C3=B8rgrav?= <des@des.no> To: Jakub Lach <jakub_lach@mailplus.pl> Cc: freebsd-hackers@freebsd.org Subject: Re: FYI Lighttpd 1.4.23 /kernel (trailing '/' on regular file symlink) vulnerability Message-ID: <86prdvipwe.fsf@ds4.des.no> In-Reply-To: <23727599.post@talk.nabble.com> (Jakub Lach's message of "Tue, 26 May 2009 10:18:50 -0700 (PDT)") References: <23727599.post@talk.nabble.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable [moving from security@ to hackers@] Jakub Lach <jakub_lach@mailplus.pl> writes: > http://www.freebsd.org/cgi/query-pr.cgi?pr=3Dkern/21768 Like bde@ pointed out, the patch is incorrect. It moves the test for v_type !=3D VDIR up to a point where, in the case of a symlink, v_type is always (by definition) VLNK. The reason why the current code does not work is that, in the symlink case, the v_type !=3D VDIR test is never reached: we will have jumped to either bad2 or success. However, it should be safe to move the test to after the success label, because trailing_slash is only ever true for the last component of the path we were asked to look up (see lines 520 through 535). The attached patch should work. DES --=20 Dag-Erling Sm=C3=B8rgrav - des@des.no --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=symlink-slash.diff Index: sys/kern/vfs_lookup.c =================================================================== --- sys/kern/vfs_lookup.c (revision 192614) +++ sys/kern/vfs_lookup.c (working copy) @@ -800,14 +800,6 @@ goto success; } - /* - * Check for bogus trailing slashes. - */ - if (trailing_slash && dp->v_type != VDIR) { - error = ENOTDIR; - goto bad2; - } - nextname: /* * Not a symbolic link. If more pathname, @@ -861,6 +853,14 @@ VOP_UNLOCK(dp, 0); success: /* + * Check for bogus trailing slashes. + */ + if (trailing_slash && dp->v_type != VDIR) { + error = ENOTDIR; + goto bad2; + } + + /* * Because of lookup_shared we may have the vnode shared locked, but * the caller may want it to be exclusively locked. */ --=-=-=--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?86prdvipwe.fsf>