Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 27 May 2009 18:46:00 +0200
From:      =?utf-8?Q?Dag-Erling_Sm=C3=B8rgrav?= <des@des.no>
To:        Bruce Evans <bde@zeta.org.au>
Cc:        freebsd-hackers@FreeBSD.org, Jakub Lach <jakub_lach@mailplus.pl>
Subject:   Re: FYI Lighttpd 1.4.23 /kernel (trailing '/' on regular file symlink) vulnerability
Message-ID:  <86r5yaijef.fsf@ds4.des.no>
In-Reply-To: <20090527233110.E4243@delplex.bde.org> (Bruce Evans's message of "Thu, 28 May 2009 01:15:17 %2B1000 (EST)")
References:  <23727599.post@talk.nabble.com> <86prdvipwe.fsf@ds4.des.no> <20090527233110.E4243@delplex.bde.org>

next in thread | previous in thread | raw e-mail | index | archive | help
--=-=-=
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable

Bruce Evans <bde@zeta.org.au> writes:
> This seems to be equivalent to the patch in the PR at the time of PR,
> except it risks breaking some other cases, so I don't see how it can
> work.

As discussed on -hackers, it doesn't.  This one does, though.

DES
--=20
Dag-Erling Sm=C3=B8rgrav - des@des.no


--=-=-=
Content-Type: text/x-patch
Content-Disposition: inline; filename=vfs_lookup-trailing-slash.diff

Index: sys/kern/vfs_lookup.c
===================================================================
--- sys/kern/vfs_lookup.c	(revision 192899)
+++ sys/kern/vfs_lookup.c	(working copy)
@@ -147,6 +147,9 @@
 		cnp->cn_flags &= ~LOCKSHARED;
 	fdp = p->p_fd;
 
+	/* We will set this ourselves if we need it. */
+	cnp->cn_flags &= ~TRAILINGSLASH;
+
 	/*
 	 * Get a buffer for the name to be translated, and copy the
 	 * name into the buffer.
@@ -533,6 +536,8 @@
 		if (*cp == '\0') {
 			trailing_slash = 1;
 			*ndp->ni_next = '\0';	/* XXX for direnter() ... */
+			if (cnp->cn_flags & ISLASTCN)
+				cnp->cn_flags |= TRAILINGSLASH;
 		}
 	}
 	ndp->ni_next = cp;
@@ -807,14 +812,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,
@@ -838,6 +835,14 @@
 		goto dirloop;
 	}
 	/*
+	 * If we're processing a path with a trailing slash,
+	 * check that the end result is a directory.
+	 */
+	if ((cnp->cn_flags & TRAILINGSLASH) && dp->v_type != VDIR) {
+		error = ENOTDIR;
+		goto bad2;
+	}
+	/*
 	 * Disallow directory write attempts on read-only filesystems.
 	 */
 	if (rdonly &&
Index: sys/sys/namei.h
===================================================================
--- sys/sys/namei.h	(revision 192900)
+++ sys/sys/namei.h	(working copy)
@@ -143,6 +143,8 @@
 #define	AUDITVNODE1	0x04000000 /* audit the looked up vnode information */
 #define	AUDITVNODE2 	0x08000000 /* audit the looked up vnode information */
 #define	PARAMASK	0x0ffffe00 /* mask of parameter descriptors */
+#define	TRAILINGSLASH	0x10000000 /* path ended in a slash */
+#define	PARAMASK	0x1ffffe00 /* mask of parameter descriptors */
 
 #define	NDHASGIANT(NDP)	(((NDP)->ni_cnd.cn_flags & GIANTHELD) != 0)
 

--=-=-=--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?86r5yaijef.fsf>