Project

General

Profile

Actions

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:
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 #1

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 #2

Updated by stbuehler over 12 years ago

  • Status changed from New to Fixed
  • % Done changed from 0 to 100

Applied in changeset r2830.

Actions #3

Updated by stbuehler over 12 years ago

  • Target version changed from 1.4.x to 1.4.31
Actions

Also available in: Atom