From: John Hodge Date: Mon, 27 Jul 2015 01:49:02 +0000 (+0800) Subject: Client - Fix #7 - Buffer oveflow when data cached across ReadLien calls X-Git-Url: https://git.ucc.asn.au/?a=commitdiff_plain;h=37e89ab2180f91b703461eb898edb9904ba6d74f;p=tpg%2Fopendispense2.git Client - Fix #7 - Buffer oveflow when data cached across ReadLien calls --- diff --git a/src/client/protocol.c b/src/client/protocol.c index fb69850..079a4ff 100644 --- a/src/client/protocol.c +++ b/src/client/protocol.c @@ -9,9 +9,11 @@ * This file is licenced under the 3-clause BSD Licence. See the file * COPYING for full details. */ +//#define DEBUG_TRACE_SERVER 2 #include #include #include +#include #include // gethostbyname #include #include @@ -1176,15 +1178,15 @@ int Dispense_SetItem(int Socket, const char *Type, int ID, int NewPrice, const c // === // Helpers // === +/// Read from the input socket until a newline is seen char *ReadLine(int Socket) { static char buf[BUFSIZ]; - static int bufPos = 0; static int bufValid = 0; - int len; + int len = 0; char *newline = NULL; int retLen = 0; - char *ret = malloc(10); + char *ret = malloc(32); #if DEBUG_TRACE_SERVER printf("ReadLine: "); @@ -1193,41 +1195,55 @@ char *ReadLine(int Socket) ret[0] = '\0'; + // While a newline hasn't been seen while( !newline ) { + assert(bufValid < BUFSIZ); + // If there is data left over from a previous call, use the data from that for the first pass if( bufValid ) { len = bufValid; + bufValid = 0; } else { - len = recv(Socket, buf+bufPos, BUFSIZ-1-bufPos, 0); + // Otherwise read some data + len = recv(Socket, buf, BUFSIZ, 0); if( len <= 0 ) { free(ret); return strdup("599 Client Connection Error\n"); } } - buf[bufPos+len] = '\0'; + assert(len < BUFSIZ); + buf[len] = '\0'; - newline = strchr( buf+bufPos, '\n' ); + // Search for newline in buffer + newline = strchr( buf, '\n' ); if( newline ) { *newline = '\0'; } - retLen += strlen(buf+bufPos); + // Increment return length by amount of data up to newline (or end of read) + retLen += strlen(buf); ret = realloc(ret, retLen + 1); - strcat( ret, buf+bufPos ); - - if( newline ) { - int newLen = newline - (buf+bufPos) + 1; - bufValid = len - newLen; - len = newLen; - } - if( len + bufPos == BUFSIZ - 1 ) bufPos = 0; - else bufPos += len; + assert(ret); // evil NULL check + strcat( ret, buf ); // append buffer data } #if DEBUG_TRACE_SERVER printf("%i '%s'\n", retLen, ret); #endif + + // If the newline wasn't the last character in the buffer. (I.e. there's extra data for the next call) + assert(newline - buf + 1 <= len); + if( newline - buf + 1 < len ) { + int extra_bytes = len - (newline - buf + 1); + // Copy `extra_bytes` from end of buffer down to start and set `bufValid` to `extra_bytes`? + memmove(&buf[0], newline + 1, extra_bytes); + bufValid = extra_bytes; + + #if DEBUG_TRACE_SERVER > 1 + printf("- Caching %i bytes '%.*s'\n", bufValid, bufValid, buf); + #endif + } return ret; }