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).
(cherry picked from commit 4b85a963be4bfc5aab9295ec627b332662f9e3b3)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit 833b224e564992847382448225eae1b6654bef70)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
(cherry picked from commit 35fb88e93d10d9a37e3dc3b8c01951690fccacb1)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
diff --git a/reg-tests/http-rules/restrict_req_hdr_names.vtc b/reg-tests/http-rules/restrict_req_hdr_names.vtc
index 57bc7e1..b493c84 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 @@
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 @@
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 @@
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 1s
@@ -114,6 +160,22 @@
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 4491ea1..e92c606 100644
--- a/src/http_ana.c
+++ b/src/http_ana.c
@@ -2655,17 +2655,21 @@
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;