tree d51384524bd78409c9da0b703e6f88d527045710
parent 0ebb511b3ea1b4b00410cedfec5c6bef47d4831a
author Willy Tarreau <w@1wt.eu> 1483537486 +0100
committer Willy Tarreau <w@1wt.eu> 1483644415 +0100
encoding latin1

BUG/MAJOR: http: fix risk of getting invalid reports of bad requests

Commits 5f10ea3 ("OPTIM: http: improve parsing performance of long
URIs") and 0431f9d ("OPTIM: http: improve parsing performance of long
header lines") introduced a bug in the HTTP parser : when a partial
request is read, the first part ends up on a 8-bytes boundary (or 4-byte
on 32-bit machines), the end lies in the header field value part, and
the buffer used to contain a CR character exactly after the last block,
then the parser could be confused and read this CR character as being
part of the current request, then switch to a new state waiting for an
LF character. Then when the next part of the request appeared, it would
read the character following what was erroneously mistaken for a CR,
see that it is not an LF and fail on a bad request. In some cases, it
can even be worse and the header following the hole can be improperly
indexed causing all sort of unexpected behaviours like a content-length
being ignored or a header appended at the wrong position. The reason is
that there's no control of and of parsing just after breaking out of the
loop.

One way to reproduce it is with this config :

  global
      stats socket /tmp/sock1 mode 666 level admin
      stats timeout 1d

  frontend  px
      bind :8001
      mode http
      timeout client 10s
      redirect location /

And sending requests this way :

  $ tcploop 8001 C P S:"$(dd if=/dev/zero bs=16384 count=1 2>/dev/null | tr '\000' '\r')"
  $ tcploop 8001 C P S:"$(dd if=/dev/zero bs=16384 count=1 2>/dev/null | tr '\000' '\r')"
  $ tcploop 8001 C P \
    S:"GET / HTTP/1.0\r\nX-padding: 0123456789.123456789.123456789.123456789.123456789.123456789.1234567" P \
    S:"89.123456789\r\n\r\n" P

Then a "show errors" on the socket will report :

  $ echo "show errors" | socat - /tmp/sock1
  Total events captured on [04/Jan/2017:15:09:15.755] : 32

  [04/Jan/2017:15:09:13.050] frontend px (#2): invalid request
    backend <NONE> (#-1), server <NONE> (#-1), event #31
    src 127.0.0.1:59716, session #91, session flags 0x00000080
    HTTP msg state 17, msg flags 0x00000000, tx flags 0x00000000
    HTTP chunk len 0 bytes, HTTP body len 0 bytes
    buffer flags 0x00808002, out 0 bytes, total 111 bytes
    pending 111 bytes, wrapping at 16384, error at position 107:

    00000  GET / HTTP/1.0\r\n
    00016  X-padding: 0123456789.123456789.123456789.123456789.123456789.12345678
    00086+ 9.123456789.123456789\r\n
    00109  \r\n

This fix must be backported to 1.7.

Many thanks to Aleksey Gordeev and Axel Reinhold for providing detailed
network captures and configurations exhibiting the issue.