Skip to content

Commit 3b69886

Browse files
committed
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!
1 parent 3d5f19e commit 3b69886

File tree

1 file changed

+6
-2
lines changed

1 file changed

+6
-2
lines changed

Diff for: include/haproxy/htx.h

+6-2
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,9 @@ static inline struct htx_blk *htx_add_header(struct htx *htx, const struct ist n
466466
{
467467
struct htx_blk *blk;
468468

469-
/* FIXME: check name.len (< 256B) and value.len (< 1MB) */
469+
if (name.len > 255 || value.len > 1048575)
470+
return NULL;
471+
470472
blk = htx_add_blk(htx, HTX_BLK_HDR, name.len + value.len);
471473
if (!blk)
472474
return NULL;
@@ -485,7 +487,9 @@ static inline struct htx_blk *htx_add_trailer(struct htx *htx, const struct ist
485487
{
486488
struct htx_blk *blk;
487489

488-
/* FIXME: check name.len (< 256B) and value.len (< 1MB) */
490+
if (name.len > 255 || value.len > 1048575)
491+
return NULL;
492+
489493
blk = htx_add_blk(htx, HTX_BLK_TLR, name.len + value.len);
490494
if (!blk)
491495
return NULL;

0 commit comments

Comments
 (0)