From 4b85a963be4bfc5aab9295ec627b332662f9e3b3 Mon Sep 17 00:00:00 2001
From: Mateusz Malek <haproxy@sl.damisa.net>
Date: Wed, 17 Aug 2022 14:22:09 +0200
Subject: [PATCH] BUG/MEDIUM: http-ana: fix crash or wrong header deletion by
 http-restrict-req-hdr-names

When using `option http-restrict-req-hdr-names delete`, HAproxy may
crash or delete wrong header after receiving request containing multiple
forbidden characters in single header name; exact behavior depends on
number of request headers, number of forbidden characters and position
of header containing them.

This patch fixes GitHub issue #1822.

Must be backported as far as 2.2 (buggy feature got included in 2.2.25,
2.4.18 and 2.5.8).
---
 .../http-rules/restrict_req_hdr_names.vtc     | 62 +++++++++++++++++++
 src/http_ana.c                                | 18 +++---
 2 files changed, 73 insertions(+), 7 deletions(-)

diff --git a/reg-tests/http-rules/restrict_req_hdr_names.vtc b/reg-tests/http-rules/restrict_req_hdr_names.vtc
index 071b9bded..4b26e33c6 100644
--- a/reg-tests/http-rules/restrict_req_hdr_names.vtc
+++ b/reg-tests/http-rules/restrict_req_hdr_names.vtc
@@ -35,6 +35,32 @@ server s5 {
     txresp
 } -start
 
+server s6 {
+    rxreq
+    expect req.http.x_my_hdr_with_lots_of_underscores  == <undef>
+    txresp
+} -start
+
+server s7 {
+    rxreq
+    expect req.http.x_my_hdr-1  == <undef>
+    expect req.http.x-my-hdr-2  == on
+    txresp
+} -start
+
+server s8 {
+    rxreq
+    expect req.http.x-my_hdr-1  == <undef>
+    expect req.http.x-my_hdr-2  == <undef>
+    txresp
+} -start
+
+server s9 {
+    rxreq
+    expect req.http.x-my-hdr-with-trailing-underscore_  == <undef>
+    txresp
+} -start
+
 haproxy h1 -conf {
     defaults
         mode http
@@ -50,6 +76,10 @@ haproxy h1 -conf {
         use_backend be-fcgi1 if { path /req4 }
         use_backend be-fcgi2 if { path /req5 }
         use_backend be-fcgi3 if { path /req6 }
+        use_backend be-http4 if { path /req7 }
+        use_backend be-http5 if { path /req8 }
+        use_backend be-http6 if { path /req9 }
+        use_backend be-http7 if { path /req10 }
 
     backend be-http1
         server s1 ${s1_addr}:${s1_port}
@@ -72,6 +102,22 @@ haproxy h1 -conf {
     backend be-fcgi3
         option http-restrict-req-hdr-names reject
 
+    backend be-http4
+        option http-restrict-req-hdr-names delete
+        server s6 ${s6_addr}:${s6_port}
+
+    backend be-http5
+        option http-restrict-req-hdr-names delete
+        server s7 ${s7_addr}:${s7_port}
+
+    backend be-http6
+        option http-restrict-req-hdr-names delete
+        server s8 ${s8_addr}:${s8_port}
+
+    backend be-http7
+        option http-restrict-req-hdr-names delete
+        server s9 ${s9_addr}:${s9_port}
+
     defaults
         mode http
         timeout connect "${HAPROXY_TEST_TIMEOUT-5s}"
@@ -114,6 +160,22 @@ client c1 -connect ${h1_fe1_sock} {
     txreq -req GET -url /req6 -hdr "X-my_hdr: on"
     rxresp
     expect resp.status == 403
+
+    txreq -req GET -url /req7 -hdr "X_my_hdr_with_lots_of_underscores: on"
+    rxresp
+    expect resp.status == 200
+
+    txreq -req GET -url /req8 -hdr "X_my_hdr-1: on" -hdr "X-my-hdr-2: on"
+    rxresp
+    expect resp.status == 200
+
+    txreq -req GET -url /req9 -hdr "X-my_hdr-1: on" -hdr "X-my_hdr-2: on"
+    rxresp
+    expect resp.status == 200
+
+    txreq -req GET -url /req10 -hdr "X-my-hdr-with-trailing-underscore_: on"
+    rxresp
+    expect resp.status == 200
 } -run
 
 client c2 -connect ${h1_fe2_sock} {
diff --git a/src/http_ana.c b/src/http_ana.c
index 4b74dd60d..2b2cfdc56 100644
--- a/src/http_ana.c
+++ b/src/http_ana.c
@@ -2645,17 +2645,21 @@ static enum rule_result http_req_restrict_header_names(struct stream *s, struct
 
 		if (type == HTX_BLK_HDR) {
 			struct ist n = htx_get_blk_name(htx, blk);
-			int i;
+			int i, end = istlen(n);
 
-			for (i = 0; i < istlen(n); i++) {
+			for (i = 0; i < end; i++) {
 				if (!isalnum((unsigned char)n.ptr[i]) && n.ptr[i] != '-') {
-					/* Block the request or remove the header */
-					if (px->options2 & PR_O2_RSTRICT_REQ_HDR_NAMES_BLK)
-						goto block;
-					blk = htx_remove_blk(htx, blk);
-					continue;
+					break;
 				}
 			}
+
+			if (i < end) {
+				/* Disallowed character found - block the request or remove the header */
+				if (px->options2 & PR_O2_RSTRICT_REQ_HDR_NAMES_BLK)
+					goto block;
+				blk = htx_remove_blk(htx, blk);
+				continue;
+			}
 		}
 		if (type == HTX_BLK_EOH)
 			break;