[Opensrf-commits] r1053 - trunk/src/srfsh

[svn at svn.open-ils.org]

Author: miker
Date: 2007-07-21 16:31:30 -0400 (Sat, 21 Jul 2007)
New Revision: 1053

Modified:
   trunk/src/srfsh/srfsh.c
Log:
Patch from Scott McKellar to fill buffer overflow holes:

The first overflow can happen with an excessively long username.

The second overflow is more doubtful, because the inputs come from
two other functions.  It's not obvious whether an overflow is possible
or not.  It may be that those functions will never return strings long
enough to overflow.  However it is easier to assume that they might,
and avoid the overflow for sure, than to determine whether an overflow
is possible in the first place.

In each case I declared a variable-length character array with a
calculated length.



Modified: trunk/src/srfsh/srfsh.c
===================================================================
--- trunk/src/srfsh/srfsh.c	2007-07-21 18:59:31 UTC (rev 1052)
+++ trunk/src/srfsh/srfsh.c	2007-07-21 20:31:30 UTC (rev 1053)
@@ -187,20 +187,6 @@
 	return 0;
 }
 
-/*
-static void sig_child_handler( int s ) {
-	child_dead = 1;
-}
-*/
-
-/*
-void sig_int_handler( int s ) {
-	printf("\n");
-	caught_sigint = 1;
-	signal(SIGINT,sig_int_handler);
-}
-*/
-
 static int load_history( void ) {
 
 	char* home = getenv("HOME");
@@ -363,16 +349,14 @@
 		int orgloci = (orgloc) ? atoi(orgloc) : 0;
 		if(!type) type = "opac";
 
-		char buf[256];
-		memset(buf,0,256);
+		char login_text[] = "request open-ils.auth open-ils.auth.authenticate.init \"%s\"";
+		size_t len = sizeof( login_text ) + strlen(username);
 
-		char buf2[256];
-		memset(buf2,0,256);
+		char buf[len];
+		buf[0] = '\0';
+		sprintf( buf, login_text, username );
+		parse_request(buf);
 
-		sprintf( buf, 
-				"request open-ils.auth open-ils.auth.authenticate.init \"%s\"", username );
-		parse_request(buf); 
-
 		char* hash;
 		if(last_result && last_result->_result_content) {
 			jsonObject* r = last_result->_result_content;
@@ -382,19 +366,13 @@
 
 		char* pass_buf = md5sum(password);
 
-		char both_buf[256];
-		memset(both_buf,0,256);
+		size_t both_len = strlen( hash ) + strlen( pass_buf ) + 1;
+		char both_buf[both_len];
+		both_buf[0] = '\0';
 		sprintf(both_buf,"%s%s",hash, pass_buf);
 
 		char* mess_buf = md5sum(both_buf);
 
-		/*
-		sprintf( buf2, "request open-ils.auth open-ils.auth.authenticate.complete "
-				"{ \"username\" : \"%s\", \"password\" : \"%s\", "
-				"\"type\" : \"%s\", \"org\" : %d, \"workstation\": \"%s\"}", 
-				username, mess_buf, type, orgloci, workstation );
-				*/
-
 		growing_buffer* argbuf = buffer_init(64);
 		buffer_fadd(argbuf, 
 				"request open-ils.auth open-ils.auth.authenticate.complete "