Commit 53612c10 authored by Per Cederqvist's avatar Per Cederqvist
Browse files

Fixed possible memory leak in mux_parse_string. This needs more work.

parent 13b4e7a1
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include <stdarg.h> #include <stdarg.h>
#include <string.h> #include <string.h>
#include "lyskomd.h" #include "lyskomd.h"
#include "s-string.h"
#include <kom-types.h> #include <kom-types.h>
#include "com.h" #include "com.h"
#include "connections.h" #include "connections.h"
...@@ -19,7 +20,7 @@ ...@@ -19,7 +20,7 @@
#include "mux-parse.h" #include "mux-parse.h"
#include "minmax.h" #include "minmax.h"
#include "config.h" #include "config.h"
#include <s-string.h> #include "log.h"
jmp_buf mux_parse_env; jmp_buf mux_parse_env;
...@@ -64,7 +65,11 @@ mux_parse_long(Mux *mux) ...@@ -64,7 +65,11 @@ mux_parse_long(Mux *mux)
/* /*
* Parse a string. At most 'maxlen' characters are allowed. If the * Parse a string. At most 'maxlen' characters are allowed. If the
* mux sends a longer string only the first 'maxlen+1' characters * mux sends a longer string only the first 'maxlen+1' characters
* are read. The following characters are discarded. * are read. Any remaining characters are discarded.
*
* longjmp if not enough characters are available or if there
* is an error. Result is used to save state in that case.
* +++ Result should not be used to save state.
*/ */
static void static void
mux_parse_string(Mux *mux, mux_parse_string(Mux *mux,
...@@ -75,16 +80,16 @@ mux_parse_string(Mux *mux, ...@@ -75,16 +80,16 @@ mux_parse_string(Mux *mux,
String_size mux_len; /* The len the mux is sending. */ String_size mux_len; /* The len the mux is sending. */
String_size truncated_len; /* How much the server will receive. */ String_size truncated_len; /* How much the server will receive. */
String_size to_skip; String_size to_skip;
String tmp; static u_long err_cnt = 0;
switch ( mux->parse.string_parse_pos ) switch ( mux->parse.string_parse_pos )
{ {
case 0: case 0:
/* Get number and discard trailing 'H' */ /* Get number and discard trailing 'H' */
result->len = s_strtol(s_fsubstr(mux->parse.unparsed, mux_len = s_strtol(s_fsubstr(mux->parse.unparsed,
mux->parse.first_to_parse, mux->parse.first_to_parse,
END_OF_STRING), END_OF_STRING),
&hptr, PROTOCOL_NUMBER_BASE); &hptr, PROTOCOL_NUMBER_BASE);
if ( hptr == -1 if ( hptr == -1
|| mux->parse.first_to_parse + hptr || mux->parse.first_to_parse + hptr
...@@ -97,6 +102,8 @@ mux_parse_string(Mux *mux, ...@@ -97,6 +102,8 @@ mux_parse_string(Mux *mux,
a) there is a trailing H a) there is a trailing H
b) there was at least one digit before the H */ b) there was at least one digit before the H */
/* +++ Use s-string.c to extract the char. */
/* +++ Fix prot_a_parse_string also. */
if ( mux->parse.unparsed.string[ mux->parse.first_to_parse if ( mux->parse.unparsed.string[ mux->parse.first_to_parse
+ hptr ] != 'H' + hptr ] != 'H'
|| hptr <= 0 ) || hptr <= 0 )
...@@ -111,8 +118,7 @@ mux_parse_string(Mux *mux, ...@@ -111,8 +118,7 @@ mux_parse_string(Mux *mux,
/* Check that the entire string is transmitted. */ /* Check that the entire string is transmitted. */
/* (Don't care about the trailing part that will be skipped if the /* (Don't care about the trailing part that will be skipped if the
* string is longer than maxlen) */ * string is longer than maxlen) */
truncated_len = min(maxlen + 1, result->len); truncated_len = min(maxlen + 1, mux_len);
mux_len = result->len;
if ( mux->parse.first_to_parse + truncated_len if ( mux->parse.first_to_parse + truncated_len
> s_strlen(mux->parse.unparsed) ) > s_strlen(mux->parse.unparsed) )
...@@ -120,35 +126,53 @@ mux_parse_string(Mux *mux, ...@@ -120,35 +126,53 @@ mux_parse_string(Mux *mux,
longjmp(mux_parse_env, MUX_MSG_INCOMPLETE); longjmp(mux_parse_env, MUX_MSG_INCOMPLETE);
} }
result->string = (mux->parse.unparsed.string if ( ( result->len != 0 || result->string != NULL) && err_cnt++ < 20 )
+ mux->parse.first_to_parse); {
result->len = truncated_len; log ("mux_parse_string(): result->len == %lu, "
tmp = EMPTY_STRING; "result->string == %lu. This memory will not be free()'d.\n",
s_strcpy(&tmp, *result); /* Copy the string. */ (u_long)result->len, (u_long)result->string);
*result = tmp; *result = EMPTY_STRING;
result->len = mux_len; if ( err_cnt == 20 )
log("Won't log the above warning no more.");
}
s_mem_crea_str(result,
mux->parse.unparsed.string + mux->parse.first_to_parse,
truncated_len);
mux->parse.first_to_parse += truncated_len; mux->parse.first_to_parse += truncated_len;
mux->parse.string_parse_pos = 2; mux->parse.string_parse_pos = 2;
result->len = mux_len; /* Can't transfer the local mux_len across
* call boundary. +++ There should be room
* for mux_len in the per-connection
* protocol data.
*/
/* Fall through */ /* Fall through */
case 2: case 2:
/* Was the string too long? If so, skip the truncated data. */ /* Was the string too long? If so, skip the truncated data. */
mux_len = result->len; mux_len = result->len; /* +++ shouldn't modify ->len */
truncated_len = min(maxlen+1, result->len); truncated_len = min(maxlen+1, result->len);
if ( mux_len > truncated_len ) if ( mux_len > truncated_len )
{ {
to_skip = min(mux_len - truncated_len, to_skip = min(mux_len - truncated_len,
mux->parse.unparsed.len - mux->parse.first_to_parse); mux->parse.unparsed.len - mux->parse.first_to_parse);
/* From now on, mux_len is length of result + numer of
chars to skip. Clean up this! +++
*/
mux_len -= to_skip; mux_len -= to_skip;
mux->parse.first_to_parse += to_skip; mux->parse.first_to_parse += to_skip;
} }
result->len = mux_len; result->len = mux_len;
if ( mux_len > truncated_len ) if ( mux_len > truncated_len )
{
/* +++ mux_len is transferred in result->len. */
longjmp(mux_parse_env, MUX_MSG_INCOMPLETE); longjmp(mux_parse_env, MUX_MSG_INCOMPLETE);
}
/* Fall through */ /* Fall through */
default: default:
mux->parse.string_parse_pos = 0; mux->parse.string_parse_pos = 0;
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment