Subject: Re: rm(1) and cp(1) printable characters diff
To: None <tech-userlevel@netbsd.org>
From: Jan Schaumann <jschauma@netbsd.org>
List: tech-userlevel
Date: 07/21/2003 23:03:15
--zx4FCpZtqtKETZ7O
Content-Type: multipart/mixed; boundary="ew6BAiZeqk4r7MaW"
Content-Disposition: inline


--ew6BAiZeqk4r7MaW
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

Charles Blundell <cb@kittenz.org> wrote:
> on Mon, Jul 21, 2003 at 10:21:18AM -0400, Jan Schaumann wrote:
> > > For what it's worth, this will overflow for certain large lengths of=
=20
> > > src. I think the magic values start at strlen(src) =3D SIZE_T_MAX/4.
> > > [(4*SIZE_T_MAX/4) + 1 =3D SIZE_T_MAX + 1 -> int overflow.]
> > > This will result in less memory being allocated than is expected
> > > when using gcc.
> >=20
> > Well, but given that SIZE_T_MAX >> MAXPATHLEN, this should never occur,
> > right?
>=20
> I see the below code where, explicitly, strlen(src) > MAXPATHLEN.
> There does not appear to be any such assertion within rm.c,
> so it seems strlen(src) > MAXPATHLEN is possible. Likewise
> there is no SIZE_T_MAX/strlen(src) >=3D 4 check. i.e. no upper
> limit is imposed on strlen(src) in these cases, except by something
> platform dependent.
>=20
> It is entirely possible that this function may be used in future=20
> where input is accepted from a source with weaker platform dependent
> limits, e.g., via stdin or a disk file.

Fair 'nuff.  So hopefully this is the last round, then: [attached].

Thanks for all feedback,
-Jan

--=20
Wenn ich tot bin, mir soll mal Einer mit Auferstehung oder so
kommen, ich hau ihm eine rein! (Anonym)

--ew6BAiZeqk4r7MaW
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=diff
Content-Transfer-Encoding: quoted-printable

Index: cp/cp.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /cvsroot/src/bin/cp/cp.c,v
retrieving revision 1.32
diff -u -r1.32 cp.c
--- cp/cp.c	16 Dec 2002 14:44:14 -0000	1.32
+++ cp/cp.c	22 Jul 2003 02:14:58 -0000
@@ -77,6 +77,7 @@
 #include <stdio.h>
 #include <string.h>
 #include <unistd.h>
+#include <vis.h>
=20
 #include "extern.h"
=20
@@ -88,14 +89,15 @@
 PATH_T to =3D { to.p_path, "" };
=20
 uid_t myuid;
-int Rflag, fflag, iflag, pflag, rflag, vflag;=20
+int Rflag, fflag, iflag, pflag, rflag, stdin_ok, vflag;=20
 mode_t myumask;
=20
 enum op { FILE_TO_FILE, FILE_TO_DIR, DIR_TO_DNE };
=20
-int main(int, char *[]);
-int copy(char *[], enum op, int);
-int mastercmp(const FTSENT **, const FTSENT **);
+int 	main(int, char *[]);
+int 	copy(char *[], enum op, int);
+int 	mastercmp(const FTSENT **, const FTSENT **);
+char	*printescaped(const char *);
=20
 int
 main(int argc, char *argv[])
@@ -153,13 +155,15 @@
 	if (argc < 2)
 		usage();
=20
+	stdin_ok =3D isatty(STDIN_FILENO);
+
 	fts_options =3D FTS_NOCHDIR | FTS_PHYSICAL;
 	if (rflag) {
 		if (Rflag)
-			errx(1,
+			errx(EXIT_FAILURE,
 		    "the -R and -r options may not be specified together.");
 		if (Hflag || Lflag || Pflag)
-			errx(1,
+			errx(EXIT_FAILURE,
 	"the -H, -L, and -P options may not be specified with the -r option.");
 		fts_options &=3D ~FTS_PHYSICAL;
 		fts_options |=3D FTS_LOGICAL;
@@ -184,8 +188,10 @@
=20
 	/* Save the target base in "to". */
 	target =3D argv[--argc];
-	if (strlen(target) > MAXPATHLEN)
-		errx(1, "%s: name too long", target);
+	if (strlen(target) > MAXPATHLEN) {
+		errx(EXIT_FAILURE, "%s: name too long", printescaped(target));
+		/* NOTREACHED */
+	}
 	(void)strcpy(to.p_path, target);
 	to.p_end =3D to.p_path + strlen(to.p_path);
         if (to.p_path =3D=3D to.p_end) {
@@ -262,7 +268,7 @@
 	FTS *ftsp;
 	FTSENT *curr;
 	int base, dne, nlen, rval;
-	char *p, *tmp;
+	char *p, *tmp, *fn;
=20
 	base =3D 0;	/* XXX gcc -Wuninitialized (see comment below) */
=20
@@ -273,12 +279,15 @@
 		case FTS_NS:
 		case FTS_DNR:
 		case FTS_ERR:
-			warnx("%s: %s",
-			    curr->fts_path, strerror(curr->fts_errno));
+			fn =3D printescaped(curr->fts_path);
+			warnx("%s: %s", fn, strerror(curr->fts_errno));
+			free(fn);
 			rval =3D 1;
 			continue;
 		case FTS_DC:			/* Warn, continue. */
-			warnx("%s: directory causes a cycle", curr->fts_path);
+			fn =3D printescaped(curr->fts_path);
+			warnx("%s: directory causes a cycle", fn);
+			free(fn);
 			rval =3D 1;
 			continue;
 		}
@@ -290,8 +299,12 @@
 		if (type !=3D FILE_TO_FILE) {
 			if ((curr->fts_namelen +
 			    to.target_end - to.p_path + 1) > MAXPATHLEN) {
-				warnx("%s/%s: name too long (not copied)",=20
-				    to.p_path, curr->fts_name);
+				char *tn;
+				tn =3D printescaped(to.p_path);
+				fn =3D printescaped(curr->fts_name);
+				warnx("%s/%s: name too long (not copied)", tn, fn);
+				free(fn);
+				free(tn);
 				rval =3D 1;
 				continue;
 			}
@@ -455,8 +468,14 @@
 				rval =3D 1;
 			break;
 		}
-		if (vflag)
-			(void)printf("%s -> %s\n", curr->fts_path, to.p_path);
+		if (vflag) {
+			char *tn;
+			fn =3D printescaped(curr->fts_path);
+			tn =3D printescaped(to.p_path);
+			(void)printf("%s -> %s\n", fn, tn);
+			free(fn);
+			free(tn);
+		}
 	}
 	if (errno)
 		err(1, "fts_read");
@@ -487,4 +506,23 @@
 	if (b_info =3D=3D FTS_D)
 		return (1);
 	return (0);
+}
+
+char *
+printescaped(const char *src)
+{
+	size_t len;
+	char *retval;
+
+	len =3D strlen(src);
+	if (len !=3D 0 && SIZE_T_MAX/len <=3D 4)
+		errx(EXIT_FAILURE, "Name too long: %s\n", src);
+
+	retval =3D (char *)malloc(4*len+1);
+	if (stdin_ok && (retval !=3D NULL)) {
+		(void)strvis(retval, src, VIS_NL | VIS_CSTYLE);
+		return retval;
+	} else=20
+		errx(EXIT_FAILURE, "Out of memory!\n");
+	/* NOTREACHED */
 }
Index: rm/rm.c
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
RCS file: /cvsroot/src/bin/rm/rm.c,v
retrieving revision 1.34
diff -u -r1.34 rm.c
--- rm/rm.c	1 Mar 2003 07:57:33 -0000	1.34
+++ rm/rm.c	22 Jul 2003 02:14:59 -0000
@@ -47,25 +47,28 @@
 #endif
 #endif /* not lint */
=20
-#include <sys/types.h>
+#include <sys/param.h>
 #include <sys/stat.h>
+#include <sys/types.h>
=20
-#include <locale.h>
 #include <err.h>
 #include <errno.h>
 #include <fcntl.h>
 #include <fts.h>
 #include <grp.h>
+#include <locale.h>
 #include <pwd.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#include <vis.h>
=20
 int dflag, eval, fflag, iflag, Pflag, stdin_ok, vflag, Wflag;
=20
 int	check(char *, char *, struct stat *);
 void	checkdot(char **);
+char 	*printescaped(const char *);
 void	rm_file(char **);
 void	rm_overwrite(char *, struct stat *);
 void	rm_tree(char **);
@@ -153,6 +156,7 @@
 	FTS *fts;
 	FTSENT *p;
 	int flags, needstat, rval;
+	char *fn;
=20
 	/*
 	 * Remove a file hierarchy.  If forcing removal (-f), or interactive
@@ -178,13 +182,15 @@
 		switch (p->fts_info) {
 		case FTS_DNR:
 			if (!fflag || p->fts_errno !=3D ENOENT) {
-				warnx("%s: %s",
-				    p->fts_path, strerror(p->fts_errno));
+				fn =3D printescaped(p->fts_path);
+				warnx("%s: %s", fn, strerror(p->fts_errno));
+				free(fn);
 				eval =3D 1;
 			}
 			continue;
 		case FTS_ERR:
-			errx(1, "%s: %s", p->fts_path, strerror(p->fts_errno));
+			errx(EXIT_FAILURE, "%s: %s", printescaped(p->fts_path),
+			    strerror(p->fts_errno));
 			/* NOTREACHED */
 		case FTS_NS:
 			/*
@@ -194,8 +200,9 @@
 			if (fflag && NONEXISTENT(p->fts_errno))
 				continue;
 			if (needstat) {
-				warnx("%s: %s",
-				    p->fts_path, strerror(p->fts_errno));
+				fn =3D printescaped(p->fts_path);
+				warnx("%s: %s", fn, strerror(p->fts_errno));
+				free(fn);
 				eval =3D 1;
 				continue;
 			}
@@ -262,7 +269,7 @@
 {
 	struct stat sb;
 	int rval;
-	char *f;
+	char *f, *fn;
=20
 	/*
 	 * Remove a file.  POSIX 1003.2 states that, by default, attempting
@@ -275,19 +282,25 @@
 				sb.st_mode =3D S_IFWHT|S_IWUSR|S_IRUSR;
 			} else {
 				if (!fflag || !NONEXISTENT(errno)) {
-					warn("%s", f);
+					fn =3D printescaped(f);
+					warn("%s", fn);
+					free(fn);
 					eval =3D 1;
 				}
 				continue;
 			}
 		} else if (Wflag) {
-			warnx("%s: %s", f, strerror(EEXIST));
+			fn =3D printescaped(f);
+			warnx("%s: %s", fn, strerror(EEXIST));
+			free(fn);
 			eval =3D 1;
 			continue;
 		}
=20
 		if (S_ISDIR(sb.st_mode) && !dflag) {
-			warnx("%s: is a directory", f);
+			fn =3D printescaped(f);
+			warnx("%s: is a directory", fn);
+			free(fn);
 			eval =3D 1;
 			continue;
 		}
@@ -368,11 +381,14 @@
 {
 	int ch, first;
 	char modep[15];
+	char *fn;
=20
 	/* Check -i first. */
-	if (iflag)
-		(void)fprintf(stderr, "remove %s? ", path);
-	else {
+	if (iflag) {
+		fn =3D printescaped(path);
+		(void)fprintf(stderr, "remove '%s'? ", fn);
+		free(fn);
+	} else {
 		/*
 		 * If it's not a symbolic link and it's unwritable and we're
 		 * talking to a terminal, ask.  Symbolic links are excluded
@@ -383,10 +399,12 @@
 		    !(access(name, W_OK) && (errno !=3D ETXTBSY)))
 			return (1);
 		strmode(sp->st_mode, modep);
-		(void)fprintf(stderr, "override %s%s%s/%s for %s? ",
+		fn =3D  printescaped(path);
+		(void)fprintf(stderr, "override %s%s%s/%s for '%s'? ",
 		    modep + 1, modep[9] =3D=3D ' ' ? "" : " ",
 		    user_from_uid(sp->st_uid, 0),
-		    group_from_gid(sp->st_gid, 0), path);
+		    group_from_gid(sp->st_gid, 0), fn);
+		free(fn);
 	}
 	(void)fflush(stderr);
=20
@@ -434,6 +452,25 @@
 		} else
 			++t;
 	}
+}
+
+char *
+printescaped(const char *src)
+{
+	size_t len;
+	char *retval;
+
+	len =3D strlen(src);
+	if (len !=3D 0 && SIZE_T_MAX/len <=3D 4)
+		errx(EXIT_FAILURE, "Name too long: %s\n", src);
+
+	retval =3D (char *)malloc(4*len+1);
+	if (stdin_ok && (retval !=3D NULL)) {
+		(void)strvis(retval, src, VIS_NL | VIS_CSTYLE);
+		return retval;
+	} else=20
+		errx(EXIT_FAILURE, "Out of memory!\n");
+	/* NOTREACHED */
 }
=20
 void

--ew6BAiZeqk4r7MaW--

--zx4FCpZtqtKETZ7O
Content-Type: application/pgp-signature
Content-Disposition: inline

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

iD8DBQE/HKlzfFtkr68iakwRAsG4AJ94tWZnLCXM7S3cKo9wkmIE794/+gCfZHGv
S35RCgQgOT1ID2l6t+aidR4=
=nmpR
-----END PGP SIGNATURE-----

--zx4FCpZtqtKETZ7O--