66 lines
2.4 KiB
Diff
66 lines
2.4 KiB
Diff
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(-)
|
|
|
|
--- a/src/htx.c
|
|
+++ b/src/htx.c
|
|
@@ -859,7 +859,9 @@ struct htx_blk *htx_add_header(struct ht
|
|
{
|
|
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;
|
|
@@ -878,7 +880,9 @@ struct htx_blk *htx_add_trailer(struct h
|
|
{
|
|
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;
|