Bug #2413: off-by-one when parsing Connection header - Lighttpd - lighty labs

Actions

Copy link

Bug #2413

closed

off-by-one when parsing Connection header

Added by pmarinescu over 12 years ago. Updated over 12 years ago.

Status:

Fixed

Priority:

High

Category:

core

Target version:

1.4.31

ASK QUESTIONS IN Forums:

Description

Valgrind reports:

==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.

Paul

Actions

Copy link

Updated by stbuehler over 12 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)

Actions

Copy link