[BusyBox] [SECURITY] potential buffer overflows caused by my_getgrgid() and my_getpwuid
Tito
farmatito at tiscali.it
Mon Aug 23 22:19:06 UTC 2004
Hi Erik,
Hi to all,
I just want to expose a problem I've found about the use of my_getgrgid and my_getpwuid in libbb.
These two functions potentially could cause buffer overflows or other troubles.
Two examples are:
1) the use of my_getpwuid() in procps_scan() function which affects at least the ps applet.
A quick demonstration is:
#adduser pppppppppppppppppp (18 chars, less will do it also)
#su pppppppppppppppppp
#e3
in another term run ./busybox ps
3547 postfix 1136 S pickup -l -t fifo -u -c -o content_filter -o receive_override
3548 postfix 1168 S qmgr -l -t fifo -u -c
3549 postfix 1516 S tlsmgr -l -t fifo -u -c
3554 root 1008 S su pppppppppppppppppp
-->3555 pppppppppS pppp 1564 S pppp bash
--> 3588 pppppppppS pppp 32 S pppp e3
This is due to the fact that the data are copied to char user[9] in
typedef struct {
int pid;
char user[9];
char state[4];
unsigned long rss;
int ppid;
#ifdef FEATURE_CPU_USAGE_PERCENTAGE
unsigned pcpu;
unsigned long stime, utime;
#endif
char *cmd;
/* basename of executable file in call to exec(2),
size from kernel headers */
char short_cmd[16];
} procps_status_t;
but my_getpwuid doesn't know the size of the buffer and passes on the whole data.
2)Another example with the id applet, where this is causing a bad behaviour rather than an overflow
with the real id we get:
#id pppppppppppppppppp
uid=502(pppppppppppppppppp) gid=504(pppppppppppppppppp) groups=504(pppppppppppppppppp)
with busybox
#./busybox id pppppppppppppppppp
id: unknown user name: pppppppp
The cause of this problems is in the use of the strcpy function, but also sprintf maybe could cause an oveflow:
char * my_getpwuid(char *name, long uid)
{
struct passwd *myuser;
myuser = getpwuid(uid);
if (myuser==NULL) {
/* maybe this can oveflow too ? */
sprintf(name, "%ld", (long)uid);
return NULL;
} else {
/* Here is the trouble */
return strcpy(name, myuser->pw_name);
}
}
Same thing for my_getgrgid
char * my_getgrgid(char *group, long gid)
{
struct group *mygroup;
mygroup = getgrgid(gid);
if (mygroup==NULL) {
sprintf(group, "%ld", gid);
return NULL;
} else {
return strcpy(group, mygroup->gr_name);
}
}
For this one I've no demostrations of overflows yet as I'haven't looked for.
Sadly I couldn't figure out a patch so far.
Two possible solutions would be:
A) use xmalloc and strlen to allocate a big enough buffers and return a pointer to the buffer
but this would break existing code and would need to add free() calls to avoid memory leaks.
A difference with the original code is that the function would not return NULL upon error.
char * my_getpwuid( long uid)
{
struct passwd *myuser;
char *name=NULL;
myuser = getpwuid(uid);
if (myuser==NULL) {
/* maybe this can oveflow too ? */
name=(char *)xmalloc(size big enough for the digits of a long int +1);
sprintf(name, "%ld", (long)uid);
} else {
/* Here is the trouble */
name=(char *)xmalloc(strlen(myuser->pw_name) +1);
sprintf(name,"%s", myuser->pw_name);
}
return name;
}
B) If in busybox systems usernames and groupnames should have a max length of 8 chars (unlike real linux systems)
than we could use safe_strncpy.
But there would always be the sprintf(name, "%ld", (long)uid) problem if an 8 char buffer is not big enough.
So maybe I'm totally wrong ;-) but now it is a trouble of the gurus to look at this.
Ciao,
Tito
More information about the busybox
mailing list