From 7e07c379fd002cbbe97f8b2a3e9b52df9b6a83be Mon Sep 17 00:00:00 2001 From: Tim Jenness Date: Fri, 14 Sep 2012 21:18:49 -0700 Subject: [PATCH] pcs: Try to trap ADAM_USER buffer overflow 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. --- libraries/pcs/msp/msp.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/libraries/pcs/msp/msp.c b/libraries/pcs/msp/msp.c index d49f92bf916..fbe0e2e6671 100644 --- a/libraries/pcs/msp/msp.c +++ b/libraries/pcs/msp/msp.c @@ -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+/+_) */ static char adam_user[MAXPATH]; /* directory for creating msp files */ @@ -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); } @@ -1262,7 +1262,12 @@ int *status /* global status (given and returned) */ /* create task rendezvous file with a name of the form ADAM_USER/_ */ - 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 */