File 0001-2.0-2.3-BUG-MAJOR-htx-fix-missing-header-n... of Package haproxy.21224

Overview Repositories Revisions Requests Users Attributes Meta

File 0001-2.0-2.3-BUG-MAJOR-htx-fix-missing-header-name-length-check-i.patch of Package haproxy.21224

From 86f4f281efb933900ebcc4fdaef95f566382d907 Mon Sep 17 00:00:00 2001
From: Willy Tarreau <w@1wt.eu>
Date: Thu, 26 Aug 2021 16:23:37 +0200
Subject: BUG/MAJOR: htx: fix missing header name length check in
 htx_add_header/trailer

Shachar Menashe for 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.

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 Shachar for his work and his responsible report!

[wt: code is in src/htx.c in 2.3 and older]
Signed-off-by: Willy Tarreau <w@1wt.eu>
---
 src/htx.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/htx.c b/src/htx.c
index b678da58e..ff5de27fa 100644
--- a/src/htx.c
+++ b/src/htx.c
@@ -881,7 +881,9 @@ struct htx_blk *htx_add_header(struct htx *htx, const struct ist name,
 {
 	struct htx_blk *blk;
 
-	/* FIXME: check name.len (< 256B) and value.len (< 1MB) */
+	if (name.len > 255 || value.len > 1048575)
+		return NULL;
+
 	blk = htx_add_blk(htx, HTX_BLK_HDR, name.len + value.len);
 	if (!blk)
 		return NULL;
@@ -900,7 +902,9 @@ struct htx_blk *htx_add_trailer(struct htx *htx, const struct ist name,
 {
 	struct htx_blk *blk;
 
-	/* FIXME: check name.len (< 256B) and value.len (< 1MB) */
+	if (name.len > 255 || value.len > 1048575)
+		return NULL;
+
 	blk = htx_add_blk(htx, HTX_BLK_TLR, name.len + value.len);
 	if (!blk)
 		return NULL;
-- 
2.28.0

Places

File 0001-2.0-2.3-BUG-MAJOR-htx-fix-missing-header-name-length-check-i.patch of Package haproxy.21224

Places