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.
diff --git a/src/proto_http.c b/src/proto_http.c
index 357401f..a72f302 100644
--- a/src/proto_http.c
+++ b/src/proto_http.c
@@ -4462,6 +4462,11 @@
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 @@
*/
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... */