Subject: bin/18647: systrace doesn't consider the destructiveness of {base,dir}name
To: None <gnats-bugs@gnats.netbsd.org>
From: Christian Biere <christianbiere@gmx.de>
List: netbsd-bugs
Date: 10/14/2002 03:56:00
>Number:         18647
>Category:       bin
>Synopsis:       systrace doesn't consider the destructiveness of {base,dir}name
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    bin-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Oct 13 18:57:00 PDT 2002
>Closed-Date:
>Last-Modified:
>Originator:     Christian Biere
>Release:        NetBSD 1.6I
>Organization:
        
>Environment:
System: NetBSD localhost 1.6I NetBSD 1.6I (DURON) #0: Wed Oct 9 02:37:50
CEST 20 02 root@localhost:/usr/src/sys/arch/i386/compile/DURON i386
Architecture: i386 
Machine: i386

>Description:
In bin/systrace/intercept.c basename() and dirname() are used as if they
would not change their parameter. Unfortunately, this is not the case. See
section BUGS in basename(3) and dirname(3).

>How-To-Repeat:
I could not use use systrace with e.g. mozilla:
$ systrace -At mozilla
[...]
intercept_filename: filename too long.
Killed.

>Fix:
See the attached patch for an appropriate fix. I'll let {base,dir}name()
work on a strdup()'ed copy instead.
--Multipart_Mon__14_Oct_2002_03:56:00_+0200_08219000
Content-Type: text/plain;
 name="intercept.c.udif"
Content-Disposition: attachment;
 filename="intercept.c.udif"
Content-Transfer-Encoding: 7bit

Index: intercept.c
===================================================================
RCS file: /cvsroot/basesrc/bin/systrace/intercept.c,v
retrieving revision 1.6
diff -u -b -r1.6 intercept.c
--- intercept.c	2002/09/17 04:54:36	1.6
+++ intercept.c	2002/10/14 01:32:11
@@ -545,33 +545,48 @@
 	}
 
 	if (name[0] != '/') {
-		if (strlcat(cwd, "/", sizeof(cwd)) >= sizeof(cwd))
+		if (strlcat(cwd, "/", sizeof(cwd)) >= sizeof(cwd)) {
+			warn("%s: case 1", __func__);
 			goto error;
-		if (strlcat(cwd, name, sizeof(cwd)) >= sizeof(cwd))
+		}
+		if (strlcat(cwd, name, sizeof(cwd)) >= sizeof(cwd)) {
+			warn("%s: case 2", __func__);
 			goto error;
+		}		
 	} else {
-		if (strlcpy(cwd, name, sizeof(cwd)) >= sizeof(cwd))
+		if (strlcpy(cwd, name, sizeof(cwd)) >= sizeof(cwd)) {
+			warn("%s: case 3", __func__);
 			goto error;
 	}
+	}
 
 	if (userp != ICLINK_NONE) {
 		static char rcwd[2*MAXPATHLEN];
 		int failed = 0;
 
 		if (userp == ICLINK_NOLAST) {
-			char *file = basename(cwd);
+			char *file;
+			char *cwd_copy;
 
+			if ((cwd_copy = strdup(cwd)) == NULL)
+				goto error;
+			file = basename(cwd_copy);
+
 			/* Check if the last component has special meaning */
 			if (strcmp(file, ".") == 0 || strcmp(file, "..") == 0)
 				userp = ICLINK_ALL;
-			else
+			else {
+				free(cwd_copy);
 				goto nolast;
 		}
+			free(cwd_copy);
+		}
 
 		/* If realpath fails then the filename does not exist,
 		 * or we are supposed to not resolve the last component */
 		if (realpath(cwd, rcwd) == NULL) {
 			char *dir, *file;
+			char *rcwd_file, *rcwd_dir;
 			struct stat st;
 
 			if (errno != EACCES) {
@@ -583,22 +598,38 @@
 			/* Component of path could not be entered */
 			if (strlcpy(rcwd, cwd, sizeof(rcwd)) >= sizeof(rcwd))
 				goto error;
-			if ((file = basename(rcwd)) == NULL)
+			if ((rcwd_file = strdup(rcwd)) == NULL)
 				goto error;
-			if ((dir = dirname(rcwd)) == NULL)
+			if ((file = basename(rcwd_file)) == NULL) {
+				free(rcwd_file);
 				goto error;
-
+			}
+			if ((rcwd_dir = strdup(rcwd)) == NULL) {
+				free(rcwd_file);
+				goto error;
+			}
+			if ((dir = dirname(rcwd_dir)) == NULL) {
+				free(rcwd_dir);
+				free(rcwd_file);
+				goto error;
+			}
 			/* So, try again */
 			if (realpath(dir, rcwd) == NULL) {
 				failed = 1;
+				free(rcwd_dir);
+				free(rcwd_file);
 				goto out;
 			}
+			free(rcwd_dir);
 			/* If path is not "/" append a "/" */
 			if (strlen(rcwd) > 1 &&
 			    strlcat(rcwd, "/", sizeof(rcwd)) >= sizeof(rcwd))
 				goto error;
-			if (strlcat(rcwd, file, sizeof(rcwd)) >= sizeof(rcwd))
+			if (strlcat(rcwd, file, sizeof(rcwd)) >= sizeof(rcwd)) {
+				free(rcwd_file);
 				goto error;
+			}
+			free(rcwd_file);
 			/* 
 			 * At this point, filename has to exist and has to
 			 * be a directory.

--Multipart_Mon__14_Oct_2002_03:56:00_+0200_08219000--

--=.0L/pbzmi0rYN.r
Content-Type: application/pgp-signature

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.0 (NetBSD)

iD8DBQE9qiQ80KQix3oyIMcRAif9AKC0yGdvjn1CLobMs/L0zOVnCWJM2gCgsxVu
5IasdF1xqfJwvWpyLN7BwyA=
=d6xT
-----END PGP SIGNATURE-----

--=.0L/pbzmi0rYN.r--
>Release-Note:
>Audit-Trail:
>Unformatted:
 --=.0L/pbzmi0rYN.r
 Content-Type: multipart/mixed;
  boundary="Multipart_Mon__14_Oct_2002_03:56:00_+0200_08219000"
 
 
 --Multipart_Mon__14_Oct_2002_03:56:00_+0200_08219000
 Content-Type: text/plain; charset=US-ASCII
 Content-Transfer-Encoding: 7bit