Tito writes:

Hi,
I've spent the half night staring at the devilish  my_getpwuid and my_getgrgid functions
trying to find out a way to avoid actual and future potential buffer overflow problems
without breaking existing code.
Finally I've  found a not intrusive way to do this that surely doesn't break existing code
and fixes a couple of problems too.
The attached patch:
1) changes the behaviour of my_getpwuid and my_getgrgid to avoid potetntial buffer overflows
2) fixes all occurences of this function calls in tar.c , id.c , ls.c, whoami.c, logger.c, libbb.h.
3) The behaviour of tar, ls and  logger is unchanged.
4) The behavior of ps with somewhat longer usernames messing up output is fixed.
5) The only bigger change was the increasing of size of the buffers in id.c to avoid
     false negatives (unknown user: xxxxxx) with usernames longer than 8 chars.
     The value i used ( 32 chars ) was taken from the tar header ( see gname and uname).
     Maybe this buffers can be reduced a bit  ( to 16 or whatever ), this is up to you.
6) The increase of size of the binary is not so dramatic:
     size busybox
       text    data     bss     dec     hex filename
     239568    2300   36816  278684   4409c busybox
    size busybox_fixed
       text    data     bss     dec     hex filename
     239616    2300   36816  278732   440cc busybox
7) The behaviour of whoami changed:
    actually it  prints out an username cut down to the size of the buffer.
    This could be fixed by increasing the size of the buffer as in id.c or
    avoid the use of my_getpwuid and use getpwuid directly instead.
    Maybe this colud be also remain unchanged......

Please apply if you think it is ok to do so.
The diff applies on today's cvs tarball (2004-08-25).
Thanks in advance,
Ciao,
Tito

3 files changed, 6 insertions, 6 deletions

diff --git a/coreutils/id.c b/coreutils/id.c
index 602b26ec3..db8afc585 100644
--- a/coreutils/id.c
+++ b/coreutils/id.c
@@ -40,7 +40,7 @@
 
 extern int id_main(int argc, char **argv)
 {
-	char user[9], group[9];
+	char user[32], group[32];
 	long pwnam, grnam;
 	int uid, gid;
 	int flags;
@@ -64,12 +64,12 @@ extern int id_main(int argc, char **argv)
 			uid = geteuid();
 			gid = getegid();
 		}
-		my_getpwuid(user, uid);
+		my_getpwuid(user, uid, sizeof(user));
 	} else {
 		safe_strncpy(user, argv[optind], sizeof(user));
 	    gid = my_getpwnamegid(user);
 	}
-	my_getgrgid(group, gid);
+	my_getgrgid(group, gid, sizeof(group));
 
 	pwnam=my_getpwnam(user);
 	grnam=my_getgrnam(group);
diff --git a/coreutils/ls.c b/coreutils/ls.c
index a87f0ec2d..22685bcd9 100644
--- a/coreutils/ls.c
+++ b/coreutils/ls.c
@@ -683,9 +683,9 @@ static int list_single(struct dnode *dn)
 			break;
 		case LIST_ID_NAME:
 #ifdef CONFIG_FEATURE_LS_USERNAME
-			my_getpwuid(scratch, dn->dstat.st_uid);
+			my_getpwuid(scratch, dn->dstat.st_uid, sizeof(scratch));
 			printf("%-8.8s ", scratch);
-			my_getgrgid(scratch, dn->dstat.st_gid);
+			my_getgrgid(scratch, dn->dstat.st_gid, sizeof(scratch));
 			printf("%-8.8s", scratch);
 			column += 17;
 			break;
diff --git a/coreutils/whoami.c b/coreutils/whoami.c
index f93034d3a..e2a03b1e9 100644
--- a/coreutils/whoami.c
+++ b/coreutils/whoami.c
@@ -36,7 +36,7 @@ extern int whoami_main(int argc, char **argv)
 		bb_show_usage();
 
 	uid = geteuid();
-	if (my_getpwuid(user, uid)) {
+	if (my_getpwuid(user, uid, sizeof(user))) {
 		puts(user);
 		bb_fflush_stdout_and_exit(EXIT_SUCCESS);
 	}