BUG/MAJOR: http: call manage_client_side_cookies() before erasing the buffer

Jean Lubatti reported a crash on haproxy using a config involving cookies
and tarpit rules. It just happens that since 1.7-dev3 with commit 83a2c3d
("BUG/MINOR : allow to log cookie for tarpit and denied request"), function
manage_client_side_cookies() was called after erasing the request buffer in
case of a tarpit action. The problem is that this function must absolutely
not be called with an empty buffer since it moves parts of it. A typical
reproducer consists in sending :

    "GET / HTTP/1.1\r\nCookie: S=1\r\n\r\n"

On such a config :

    listen crash
        bind :8001
        mode http
        reqitarpit .
        cookie S insert indirect
        server s1 127.0.0.1:8000 cookie 1

The fix simply consists in moving the call to the function before the call
to buffer_erase().

Many thanks to Jean for testing instrumented code and providing a usable
core.

This fix must be backported to all stable versions since the fix introducing
this bug was backported as well.
This commit is contained in:
Willy Tarreau 2017-06-11 17:56:27 +02:00
parent bd6989152f
commit 6a0bca9e78

View File

@ -4462,6 +4462,11 @@ int http_process_req_common(struct stream *s, struct channel *req, int an_bit, s
return 1;
tarpit:
/* Allow cookie logging
*/
if (s->be->cookie_name || sess->fe->capture_name)
manage_client_side_cookies(s, req);
/* When a connection is tarpitted, we use the tarpit timeout,
* which may be the same as the connect timeout if unspecified.
* If unset, then set it to zero because we really want it to
@ -4474,11 +4479,6 @@ int http_process_req_common(struct stream *s, struct channel *req, int an_bit, s
*/
channel_dont_connect(req);
/* Allow cookie logging
*/
if (s->be->cookie_name || sess->fe->capture_name)
manage_client_side_cookies(s, req);
txn->status = http_err_codes[deny_status];
req->analysers &= AN_REQ_FLT_END; /* remove switching rules etc... */