summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorEric Andersen2004-08-26 22:18:59 +0000
committerEric Andersen2004-08-26 22:18:59 +0000
commit52499cb9ae01a67187c65ea43a48140b147968cf (patch)
treecf0f347d2c4a0cd48c6d5b4cc83aead35726434e
parent6fea7328ee525724309dcad46f34718fc654e083 (diff)
downloadbusybox-52499cb9ae01a67187c65ea43a48140b147968cf.zip
busybox-52499cb9ae01a67187c65ea43a48140b147968cf.tar.gz
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
-rw-r--r--archival/tar.c4
-rw-r--r--coreutils/id.c6
-rw-r--r--coreutils/ls.c4
-rw-r--r--coreutils/whoami.c2
-rw-r--r--include/libbb.h4
-rw-r--r--libbb/my_getgrgid.c6
-rw-r--r--libbb/my_getpwuid.c6
-rw-r--r--libbb/procps.c2
-rw-r--r--sysklogd/logger.c2
9 files changed, 18 insertions, 18 deletions
diff --git a/archival/tar.c b/archival/tar.c
index 689dd14..950e21d 100644
--- a/archival/tar.c
+++ b/archival/tar.c
@@ -234,9 +234,9 @@ static inline int writeTarHeader(struct TarBallInfo *tbInfo,
TAR_MAGIC_LEN + TAR_VERSION_LEN);
/* Enter the user and group names (default to root if it fails) */
- if (my_getpwuid(header.uname, statbuf->st_uid) == NULL)
+ if (my_getpwuid(header.uname, statbuf->st_uid, sizeof(header.uname)) == NULL)
strcpy(header.uname, "root");
- if (my_getgrgid(header.gname, statbuf->st_gid) == NULL)
+ if (my_getgrgid(header.gname, statbuf->st_gid, sizeof(header.gname)) == NULL)
strcpy(header.gname, "root");
if (tbInfo->hlInfo) {
diff --git a/coreutils/id.c b/coreutils/id.c
index 602b26e..db8afc5 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 a87f0ec..22685bc 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 f93034d..e2a03b1 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);
}
diff --git a/include/libbb.h b/include/libbb.h
index afbd020..78b9711 100644
--- a/include/libbb.h
+++ b/include/libbb.h
@@ -230,8 +230,8 @@ extern unsigned long bb_xparse_number(const char *numstr,
* increases target size and is often not needed embedded systems. */
extern long my_getpwnam(const char *name);
extern long my_getgrnam(const char *name);
-extern char * my_getpwuid(char *name, long uid);
-extern char * my_getgrgid(char *group, long gid);
+extern char * my_getpwuid(char *name, long uid, int bufsize);
+extern char * my_getgrgid(char *group, long gid, int bufsize);
extern long my_getpwnamegid(const char *name);
extern char *bb_askpass(int timeout, const char * prompt);
diff --git a/libbb/my_getgrgid.c b/libbb/my_getgrgid.c
index 907a474..e6b8776 100644
--- a/libbb/my_getgrgid.c
+++ b/libbb/my_getgrgid.c
@@ -27,16 +27,16 @@
/* gets a groupname given a gid */
-char * my_getgrgid(char *group, long gid)
+char * my_getgrgid(char *group, long gid, int bufsize)
{
struct group *mygroup;
mygroup = getgrgid(gid);
if (mygroup==NULL) {
- sprintf(group, "%ld", gid);
+ snprintf(group, bufsize, "%ld", gid);
return NULL;
} else {
- return strcpy(group, mygroup->gr_name);
+ return safe_strncpy(group, mygroup->gr_name, bufsize);
}
}
diff --git a/libbb/my_getpwuid.c b/libbb/my_getpwuid.c
index 21a037f..53f6c77 100644
--- a/libbb/my_getpwuid.c
+++ b/libbb/my_getpwuid.c
@@ -28,16 +28,16 @@
/* gets a username given a uid */
-char * my_getpwuid(char *name, long uid)
+char * my_getpwuid(char *name, long uid, int bufsize)
{
struct passwd *myuser;
myuser = getpwuid(uid);
if (myuser==NULL) {
- sprintf(name, "%ld", (long)uid);
+ snprintf(name, bufsize, "%ld", (long)uid);
return NULL;
} else {
- return strcpy(name, myuser->pw_name);
+ return safe_strncpy(name, myuser->pw_name, bufsize);
}
}
diff --git a/libbb/procps.c b/libbb/procps.c
index 46e9827..e405fb7 100644
--- a/libbb/procps.c
+++ b/libbb/procps.c
@@ -57,7 +57,7 @@ extern procps_status_t * procps_scan(int save_user_arg0
sprintf(status, "/proc/%d", pid);
if(stat(status, &sb))
continue;
- my_getpwuid(curstatus.user, sb.st_uid);
+ my_getpwuid(curstatus.user, sb.st_uid, sizeof(curstatus.user));
sprintf(status, "/proc/%d/stat", pid);
if((fp = fopen(status, "r")) == NULL)
diff --git a/sysklogd/logger.c b/sysklogd/logger.c
index 981cef3..1615531 100644
--- a/sysklogd/logger.c
+++ b/sysklogd/logger.c
@@ -108,7 +108,7 @@ extern int logger_main(int argc, char **argv)
char buf[1024], name[128];
/* Fill out the name string early (may be overwritten later) */
- my_getpwuid(name, geteuid());
+ my_getpwuid(name, geteuid(), sizeof(name));
/* Parse any options */
while ((opt = getopt(argc, argv, "p:st:")) > 0) {