mirror of
http://git.haproxy.org/git/haproxy.git/
synced 2025-04-18 04:55:37 +00:00
BUG/MAJOR: http: fix risk of getting invalid reports of bad requests
Commits5f10ea3
("OPTIM: http: improve parsing performance of long URIs") and0431f9d
("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.
This commit is contained in:
parent
0ebb511b3e
commit
2afff9c2d6
@ -1560,6 +1560,10 @@ const char *http_parse_reqline(struct http_msg *msg,
|
||||
ptr += sizeof(int);
|
||||
}
|
||||
#endif
|
||||
if (ptr >= end) {
|
||||
state = HTTP_MSG_RQURI;
|
||||
goto http_msg_ood;
|
||||
}
|
||||
http_msg_rquri2:
|
||||
if (likely((unsigned char)(*ptr - 33) <= 93)) /* 33 to 126 included */
|
||||
EAT_AND_JUMP_OR_RETURN(http_msg_rquri2, HTTP_MSG_RQURI);
|
||||
@ -1994,6 +1998,10 @@ void http_msg_analyzer(struct http_msg *msg, struct hdr_idx *idx)
|
||||
ptr += sizeof(int);
|
||||
}
|
||||
#endif
|
||||
if (ptr >= end) {
|
||||
state = HTTP_MSG_HDR_VAL;
|
||||
goto http_msg_ood;
|
||||
}
|
||||
http_msg_hdr_val2:
|
||||
if (likely(!HTTP_IS_CRLF(*ptr)))
|
||||
EAT_AND_JUMP_OR_RETURN(http_msg_hdr_val2, HTTP_MSG_HDR_VAL);
|
||||
|
Loading…
Reference in New Issue
Block a user