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.

git-svn-id: svn://svn.open-ils.org/OpenSRF/trunk@1053 9efc2488-bf62-4759-914b-345cdb29e865

diff --git a/src/srfsh/srfsh.c b/src/srfsh/srfsh.c
index 2d41125..e546681 100644 (file)
--- a/src/srfsh/srfsh.c
+++ b/src/srfsh/srfsh.c
@@ -187,20 +187,6 @@ int main( int argc, char* argv[] ) {
        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,15 +349,13 @@ static int handle_login( char* words[]) {
                int orgloci = (orgloc) ? atoi(orgloc) : 0;
                if(!type) type = "opac";
 
-               char buf[256];
-               memset(buf,0,256);
-
-               char buf2[256];
-               memset(buf2,0,256);
+               char login_text[] = "request open-ils.auth open-ils.auth.authenticate.init \"%s\"";
+               size_t len = sizeof( login_text ) + strlen(username);
 
-               sprintf( buf, 
-                               "request open-ils.auth open-ils.auth.authenticate.init \"%s\"", username );
-               parse_request(buf); 
+               char buf[len];
+               buf[0] = '\0';
+               sprintf( buf, login_text, username );
+               parse_request(buf);
 
                char* hash;
                if(last_result && last_result->_result_content) {
@@ -382,19 +366,13 @@ static int handle_login( char* words[]) {
 
                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 "