tree 9de63fb98007fc1120f41138f1ed85aea608cf1f
parent 3d5f19e04d88e7c8f71cba4ea12e383c91de89f6
author Willy Tarreau <w@1wt.eu> 1629987817 +0200
committer Willy Tarreau <w@1wt.eu> 1630678529 +0200
encoding latin1

BUG/MAJOR: htx: fix missing header name length check in htx_add_header/trailer

Ori Hollander of JFrog Security reported that htx_add_header() and
htx_add_trailer() were missing a length check on the header name. While
this does not allow to overwrite any memory area, it results in bits of
the header name length to slip into the header value length and may
result in forging certain header names on the input. The sad thing here
is that a FIXME comment was present suggesting to add the required length
checks :-(

The injected headers are visible to the HTTP internals and to the config
rules, so haproxy will generally stay synchronized with the server. But
there is one exception which is the content-length header field, because
it is already deduplicated on the input, but before being indexed. As
such, injecting a content-length header after the deduplication stage
may be abused to present a different, shorter one on the other side and
help build a request smuggling attack, or even maybe a response splitting
attack. CVE-2021-40346 was assigned to this problem.

As a mitigation measure, it is sufficient to verify that no more than
one such header is present in any message, which is normally the case
thanks to the duplicate checks:

   http-request  deny if { req.hdr_cnt(content-length) gt 1 }
   http-response deny if { res.hdr_cnt(content-length) gt 1 }

This must be backported to all HTX-enabled versions, hence as far as 2.0.
In 2.3 and earlier, the functions are in src/htx.c instead.

Many thanks to Ori for his work and his responsible report!