>Number:         6009
>Category:       pkg
>Synopsis:       pkg_add fails if PREFIX is a symbolic link
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    gnats-admin (GNATS administrator)
>State:          open
>Class:          change-request
>Submitter-Id:   net
>Arrival-Date:   Sun Aug 23 18:50:00 1998
>Last-Modified:
>Originator:     Jim Bernard
>Organization:
	Speaking for myself
>Release:        August 22, 1998
>Environment:
System: NetBSD zoo 1.3F NetBSD 1.3F (ZOO) #0: Sun Jun 14 09:09:08 MDT 1998 local@zoo:/home/local/compile/sys/arch/i386/compile/ZOO i386


>Description:
	If, say, /usr/X11R6 is a symbolic link, then pkg_add will fail
	to install a binary package because it stats the link, not the
	directory to which it points.  Now I _swear_ this used to work,
	but it's quite clearly broken now.

>How-To-Repeat:
	Make a binary package.  Try to install it with pkg_add, which will
	fail.

>Fix:
	The problem is that in make_hierarchy (in
	usr.sbin/pkg_install/add/futil.c) each component of the path is
	tested with the function isdir (in usr.sbin/pkg_install/lib/file.c),
	which uses lstat and tests for a directory with S_ISDIR.  This test
	fails on a symbolic link, since the returned stat structure refers
	to the link, not the directory it points to.

	Since (1) there may be other sections of the code that really want
	to use isdir to check exclusively for real directories, and (2)
	there's a (small) security risk involved in simply following
	symbolic links during an important operation like installing
	packages as root, I decided to add a second function, "islinktodir"
	to test for links to directories.  Then the test in make_hierarchy
	becomes isdir || islinktodir.

	The islinktodir function checks first that the file is a link,
	and if so, checks that the directory in which the link resides
	is owned by root or the invoking user and does not have group
	or other write permission.  If that test succeeds, it checks
	to see that the object pointed to by the link exists and is
	a directory.

	Patches are to usr.sbin/pkg_install/{lib/file.c,lib/lib.h,add/futil.c}.

--- lib/file.c-dist	Fri Jul 10 05:17:10 1998
+++ lib/file.c	Sun Aug 23 15:07:50 1998
@@ -61,10 +61,42 @@
 	return TRUE;
     else
 	return FALSE;
 }
 
+/*
+ * Check to see if something is a (safe) symbolic link to a directory.
+ * Here "safe" means that the directory containing the link is owned
+ * by root or the invoking user and does not have group or other write
+ * permission.
+ */
+Boolean
+islinktodir(char *fname)
+{
+    struct stat sb;
+    char c, *cp;
+    uid_t me;
+
+    if (lstat(fname, &sb) != FAIL && S_ISLNK(sb.st_mode)) {
+	if ((cp = strrchr(fname, '/')) == NULL || cp == fname)
+	    return FALSE;		/* No bare names or "/" as link */
+	c = *cp;
+	*cp = '\0';
+	me = geteuid();
+	if (stat(fname, &sb) != FAIL && S_ISDIR(sb.st_mode)
+	    && (sb.st_uid == 0 || sb.st_uid == me)
+	    && (sb.st_mode & (S_IWGRP|S_IWOTH)) == 0)
+	{
+	    *cp = c;
+	    if (stat(fname, &sb) != FAIL && S_ISDIR(sb.st_mode))
+		return TRUE;
+	}
+	*cp = c;
+    }
+    return FALSE;
+}
+
 /* Check to see if file is a dir, and is empty */
 Boolean
 isemptydir(char *fname)
 {
     if (isdir(fname)) {
--- lib/lib.h-dist	Mon Jul  6 05:13:39 1998
+++ lib/lib.h	Sun Aug 23 15:14:55 1998
@@ -124,10 +124,11 @@
 char		*strconcat(char *, char *);
 
 /* File */
 Boolean		fexists(char *);
 Boolean		isdir(char *);
+Boolean		islinktodir(char *);
 Boolean		isemptydir(char *fname);
 Boolean		isemptyfile(char *fname);
 Boolean         isfile(char *);
 Boolean		isempty(char *);
 Boolean		isURL(char *);
--- add/futil.c-dist	Sat Oct 18 13:40:05 1997
+++ add/futil.c	Sun Aug 23 15:10:52 1998
@@ -49,11 +49,11 @@
 	cp1 = cp2 = dir;
     while (cp2) {
 	if ((cp2 = strchr(cp1, '/')) !=NULL )
 	    *cp2 = '\0';
 	if (fexists(dir)) {
-	    if (!isdir(dir))
+	    if (!(isdir(dir) || islinktodir(dir)))
 		return FAIL;
 	}
 	else {
 	    if (vsystem("mkdir %s", dir))
 		return FAIL;
>Audit-Trail:
>Unformatted: