off-by-one when parsing Connection header
==21749== Invalid read of size 1 ==21749== at 0x412852: http_request_parse (request.c:246) ==21749== by 0x40BAD8: connection_state_machine (connections.c:1428) ==21749== by 0x407FF1: main (server.c:1507) ==21749== Address 0x53993af is 1 bytes before a block of size 64 alloc'd ==21749== at 0x4029B6A: malloc (vg_replace_malloc.c:263) ==21749== by 0x41463B: buffer_prepare_copy (buffer.c:87) ==21749== by 0x414C38: buffer_copy_string_len (buffer.c:149) ==21749== by 0x412471: http_request_parse (request.c:808) ==21749== by 0x40BAD8: connection_state_machine (connections.c:1428) ==21749== by 0x407FF1: main (server.c:1507)
when a client issues a request such as
GET / HTTP/1.0 Connection: ,whatever
The problem seems to exist in all versions.
#1 Updated by stbuehler almost 4 years ago
- Priority changed from Normal to High
Good catch, this looks very... broken. Although I think there won't anything bad happen as long as the byte before a string is mapped in memory (and I guess usually the malloc() allocator has some data in this place) - the value doesn't actually get used.
for (; (*end == ' ' || *end == '\t') && end > start; end--);
If start was > 0, and *start == ',', then end is start-1 and end < start - so end isn't decremented (swapping the arguments of && already fixes the memory access).
buffer_copy_string_len(ds->value, start, end-start+1);
end-start+1 is at least 0, so nothing bad should happen (I think the pointer difference is calculated as a signed value, so -1+1 shouldn't trigger an overflow)
Also available in: Atom