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

[svn at svn.open-ils.org]

Author: miker
Date: 2007-07-10 23:17:13 -0400 (Tue, 10 Jul 2007)
New Revision: 1022

Modified:
   trunk/src/srfsh/srfsh.c
Log:
Patch from Scott McKellar to fix potential local buffer
overflow attack against srfsh:

This patch fixes a potential buffer overflow in the parse_error
function.  The existing code concatenates the strtoked tokens into a
fixed-length buffer, with no check for overflow.  It isn't hard to
build an srfsh command that overflows the buffer, with baleful
results.

While it's not likely that anyone would do so by accident from the
command line, an srfsh script might well do so, especially if the
script were generated from another program.

More important, someone sufficiently clever might be able to use
such an overflow to work mischief.

My version of parse_error() uses a growing_buffer to accumulate
the tokens.



Modified: trunk/src/srfsh/srfsh.c
===================================================================
--- trunk/src/srfsh/srfsh.c	2007-07-11 03:15:39 UTC (rev 1021)
+++ trunk/src/srfsh/srfsh.c	2007-07-11 03:17:13 UTC (rev 1022)
@@ -24,7 +24,7 @@
 
 static char* history_file = NULL;
 
-static int child_dead = 0;
+//static int child_dead = 0;
 
 static char* login_session = NULL;
 
@@ -50,20 +50,21 @@
 
 /* handles app level requests */
 static int handle_request( char* words[], int relay );
-static int handle_exec(char* words[], int new_shell);
+//static int handle_exec(char* words[], int new_shell);
 static int handle_set( char* words[]);
 static int handle_print( char* words[]);
 static int send_request( char* server, 
 				  char* method, growing_buffer* buffer, int relay );
 static int parse_error( char* words[] );
-static int router_query_servers( char* server );
+static int router_query_servers( const char* server );
+static int print_help( void );
+
 //static int srfsh_client_connect();
-static int print_help();
 //static char* tabs(int count);
-static void sig_child_handler( int s );
+//static void sig_child_handler( int s );
 //static void sig_int_handler( int s );
 
-static int load_history();
+static int load_history( void );
 static int handle_math( char* words[] );
 static int do_math( int count, int style );
 static int handle_introspect(char* words[]);
@@ -155,9 +156,11 @@
 	return 0;
 }
 
+/*
 static void sig_child_handler( int s ) {
 	child_dead = 1;
 }
+*/
 
 /*
 void sig_int_handler( int s ) {
@@ -167,7 +170,7 @@
 }
 */
 
-static int load_history() {
+static int load_history( void ) {
 
 	char* home = getenv("HOME");
 	int l = strlen(home) + 24;
@@ -190,19 +193,15 @@
 	if( ! words )
 		return 0;
 
-
-	int i = 0;
-	char* current;
-	char buffer[256];
-	memset(buffer, 0, 256);
-	while( (current=words[i++]) ) {
-		strcat(buffer, current);
-		strcat(buffer, " ");
+	growing_buffer * gbuf = buffer_init( 64 );
+	buffer_add( gbuf, *words );
+	while( *++words ) {
+		buffer_add( gbuf, " " );
+		buffer_add( gbuf, *words );
 	}
-	if( ! buffer || strlen(buffer) < 1 ) 
-		printf("\n");
-
-	fprintf( stderr, "???: %s\n", buffer );
+	fprintf( stderr, "???: %s\n", gbuf->buf );
+	buffer_free( gbuf );
+	
 	return 0;
 
 }
@@ -213,6 +212,8 @@
 	if( request == NULL )
 		return 0;
 
+	char * original_request = strdup( request );
+	
 	int ret_val = 0;
 	int i = 0;
 	char* words[COMMAND_BUFSIZE]; 
@@ -222,7 +223,10 @@
 	char* cur_tok = strtok( req, " " );
 
 	if( cur_tok == NULL )
+	{
+		free( original_request );
 		return 0;
+	}
 
 	while(cur_tok != NULL) {
 		words[i++] = cur_tok;
@@ -266,9 +270,14 @@
 	else if (!strcmp(words[0],"login"))
 		ret_val = handle_login(words);
 
-	else if (words[0][0] == '!')
-		ret_val = handle_exec( words, 1 );
-
+	else if (words[0][0] == '!') {
+		//ret_val = handle_exec( words, 1 );
+		system( original_request + 1 );
+		ret_val = 1;
+	}
+	
+	free( original_request );
+	
 	if(!ret_val) {
 		#ifdef EXEC_DEFAULT
 			return handle_exec( words, 0 );
@@ -477,6 +486,7 @@
 
 /* if new shell, spawn a new child and subshell to do the work,
 	otherwise pipe the request to the currently open (piped) shell */
+/*
 static int handle_exec(char* words[], int new_shell) {
 
 	if(!words[0]) return 0;
@@ -486,7 +496,7 @@
 		char command[len];
 		memset(command,0,len);
 	
-		int i; /* chop out the ! */
+		int i; // chop out the !
 		for( i=1; i!= len; i++) {
 			command[i-1] = words[0][i];
 		}
@@ -521,30 +531,18 @@
 	
 		buffer_add( b, "\n");
 	
-		//int reader;
-		//int reader = dup2(STDOUT_FILENO, reader);
-		//int reader = dup(STDOUT_FILENO);
-		//close(STDOUT_FILENO);
-
 		fprintf( shell_writer, b->buf );
 		buffer_free(b);
 	
 		fflush(shell_writer);
 		usleep(1000);
 
-		/*
-		char c[4096];
-		bzero(c, 4096);
-		read( reader, c, 4095 );
-		fprintf(stderr, "read %s", c);
-		dup2(reader, STDOUT_FILENO);
-		*/
-
 	}
 
 	
 	return 1;
 }
+*/
 
 
 static int handle_request( char* words[], int relay ) {
@@ -758,7 +756,7 @@
 
 		
 
-static int router_query_servers( char* router_server ) {
+static int router_query_servers( const char* router_server ) {
 
 	if( ! router_server || strlen(router_server) == 0 ) 
 		return 0;
@@ -795,7 +793,7 @@
 	return 1;
 }
 
-static int print_help() {
+static int print_help( void ) {
 
 	printf(
 			"---------------------------------------------------------------------------------\n"