Skip to content

Commit

Permalink
pcs: Try to trap ADAM_USER buffer overflow
Browse files Browse the repository at this point in the history
ADAM_USER is being copied into a fixed size buffer. There are
traps in place for this except the buffer being used to report the
error was only 100 characters in size and was being asked to
include the ADAM_USER definition in the message.

If ADAM_USER fitted inside a buffer of size MAXPATH sprintf was then
being used to fill a buffer of size MAXPATH with the rendezvous
file name which is ADAM_USER/taskname_portnum where ADAM_USER can
be up to MAXPATH and taskname can be 32 characters. The rendezvous
buffer is now sized to accept the file name as well as ADAM_USER
and in addition the sprintf is converted to a snprintf to trap
for buffer overflow and abort if the (now 1024 but before a recent
patch was only 100) path is too long. This fixes random Aborts
from OSX if you happen to set your own ADAM_USER to just the wrong
size.
  • Loading branch information
timj committed Sep 15, 2012
1 parent 3903938 commit 7e07c37
Showing 1 changed file with 9 additions and 4 deletions.
13 changes: 9 additions & 4 deletions libraries/pcs/msp/msp.c
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ static int mysockets[MSP__MXSOCKETS]; /* list of open sockets */
static int socket_used[MSP__MXSOCKETS]; /* flags for currently-active
sockets */

static char rendezvous[MAXPATH]; /* name of rendezvous file */
static char rendezvous[MAXPATH+1+32+1+5]; /* name of rendezvous file (ADAM_USER+/+<task>_<PORT>) */

static char adam_user[MAXPATH]; /* directory for creating msp
files */
Expand Down Expand Up @@ -1182,8 +1182,8 @@ int *status /* global status (given and returned) */
Creates the directory if necessary */

if ( msp1_admus( adam_user, MAXPATH) ) {
sprintf ( string, "%s aborting, failed creating directory %s",
my_name, adam_user );
sprintf ( string, "%s aborting, failed creating or obtaining ADAM_USER directory",
my_name );
perror ( string );
exit(1);
}
Expand Down Expand Up @@ -1262,7 +1262,12 @@ int *status /* global status (given and returned) */
/* create task rendezvous file with a name of the form
ADAM_USER/<taskname>_<portnumber> */

sprintf ( rendezvous, "%s/%s_%d", adam_user, task_name, portno );
istat = snprintf ( rendezvous, sizeof(rendezvous), "%s/%s_%d", adam_user, task_name, portno );
if (istat >= sizeof(rendezvous) ) {
fprintf( stderr, "%s aborting, ADAM_USER directory (%s) is too long for internal buffer\n", my_name,
adam_user );
exit(1);
}

#if __CYGWIN32__
/* mknod and mkfifo are dummies, so test works, but cannot be used */
Expand Down

0 comments on commit 7e07c37

Please sign in to comment.