[Mod-security-developers] modsecurity_request_body_to_stream() logic

[Marc S. <mar...@ap...>]

Hello,

While looking at memory leaks inside 
modsecurity_request_body_to_stream(), I found some strange logic I 
couldn't really explain but by mistakes.
I've been looking at the code when MSC_LARGE_STREAM_INPUT is NOT defined.
Can somebody validate a few points:

1. we always allocate (msr->stream_input_length + 1) bytes (potentitally 
-buflen when relevant)
Do we really need this extra-byte (used for EOS I think)?

2. we have some memset(msr->stream_input_data, 0, ...) at several places
For me, these are only used to set a 0 at EOS (thus last byte).
We shouldn't loose time to set all bytes to 0, but I think that the last 
one is also useless.
Example, when in first packet, we have that code:
memset(msr->stream_input_data, 0, msr->stream_input_length+1);
if(first_pkt)   {
    memcpy(msr->stream_input_data, buffer, msr->stream_input_length);
}
Remark: potentially a EOS could be added at the very end of the whole 
buffer processing, if really needed - or we can add it every time if simpler

3. when not in first packet, we have that (simplified) code:
// copy beginning of msr->stream_input_data in a new buffer
data = (char *)malloc(msr->stream_input_length + 1 - buflen);
memset(data, 0, msr->stream_input_length + 1 - buflen); // only EOS 
should be enough
memcpy(data, msr->stream_input_data, msr->stream_input_length - buflen);
=> useless because msr->stream_input_data content is preserved in realloc()

// realloc msr->stream_input_data
stream_input_body = (char *)realloc(msr->stream_input_data, 
msr->stream_input_length + 1);
msr->stream_input_data = (char *)stream_input_body;

memset(msr->stream_input_data, 0, msr->stream_input_length+1); // only 
EOS should be enough
memcpy(msr->stream_input_data, data, msr->stream_input_length - buflen); 
// already done in realloc()
// append buffer (buflen)
memcpy(msr->stream_input_data+(msr->stream_input_length - buflen), 
buffer, buflen);

I think this whole code could be simplified:
stream_input_body = (char *)realloc(msr->stream_input_data, 
msr->stream_input_length + 1);
msr->stream_input_data = stream_input_body;
memcpy(msr->stream_input_data+(msr->stream_input_length - buflen), 
buffer, buflen);
=> did I miss something?