BUG/MINOR: http-ana: fix a one-byte over-read in the client-side cookie parser

In http_manage_client_side_cookies(), each iteration of the cookie loop skips
the blanks in front of the attribute name, then tests it against '$':

	while (att_beg < hdr_end && HTTP_IS_SPHT(*att_beg))
		att_beg++;
	...
	if (*att_beg == '$')
		continue;

<att_beg> may legitimately have reached <hdr_end>, in which case the test reads
one byte past the end of the Cookie header value. This happens for any value
ending with a delimiter, optionally followed by blanks, e.g. "Cookie: a=b; ":
the last iteration starts on the ';', skips it and the trailing space, and lands
exactly on <hdr_end>.

The extra byte always belongs to the HTX buffer (the payload area is followed by
other blocks, the free space or the block table), so there is no out-of-bounds
access at the allocation level, and the only functional consequence is that a
neighbour byte holding a '$' makes the parser skip the end of the header as if
it were an attribute instead of marking the header as to be preserved. But the
value must not be read past its end, and the check is free since <hdr_end> is
already at hand.

Note that http_manage_server_side_cookies() does not have this issue, it uses
"equal == val_end" to detect the empty trailing element.

This has been there since the HTX rewrite of the analysers by commit f4eb75d17
("MINOR: htx: Add proto_htx.c file") in 1.9-dev7, and the pre-HTX code had the
same construct, so it should be backported to all supported versions.

diff --git a/src/http_ana.c b/src/http_ana.c
index b4a57cb082267730cf29f44742ed46059ad97ee0..41eaf68734310048b0f73e8a009438edf757c9a1 100644 (file)
--- a/src/http_ana.c
+++ b/src/http_ana.c
@@ -3341,9 +3341,12 @@ static void http_manage_client_side_cookies(struct stream *s, struct channel *re
                        /* We have nothing to do with attributes beginning with
                         * '$'. However, they will automatically be removed if a
                         * header before them is removed, since they're supposed
-                        * to be linked together.
+                        * to be linked together. Note that <att_beg> may be equal
+                        * to <hdr_end> for a header value ending with a delimiter
+                        * possibly followed by blanks, so it must not be
+                        * dereferenced without being checked first.
                         */
-                       if (*att_beg == '$')
+                       if (att_beg < hdr_end && *att_beg == '$')
                                continue;
 
                        /* Ignore cookies with no equal sign */