From: Christopher Faulet Date: Thu, 17 Sep 2026 06:44:05 +0000 (+0200) Subject: BUG/MAJOR: htx: Check the header/trailer length limits when one is updated X-Git-Tag: v3.5-dev8~177 X-Git-Url: https://git.kaiwu.me/?a=commitdiff_plain;h=89f19bea8c1e05947d206a38883d2107bc5fd824;p=haproxy.git BUG/MAJOR: htx: Check the header/trailer length limits when one is updated The name and the value lengths of a header or a trailer are respectively encoded on 8 and 20 bits in the block descriptor. So the name must not exceed 255 bytes and the value must not exceed 1048575 bytes. This is properly tested when a header or a trailer is added, since commit 3b69886f7 ("BUG/MAJOR: htx: fix missing header name length check in htx_add_header/trailer"). But no test was performed when one is updated. htx_replace_blk_value(), used to update a part of a header or a trailer value, may grow it above the limit. And htx_replace_header(), used to replace a full header, does not check its arguments at all. In both cases, the length silently overflows on the block type, changing it. For instance, a HDR block becomes an EOH block while its value length wraps to a small value. Only the value length limit is reachable from the configuration, via a "replace-header" or a "replace-value" rule, and only if "tune.bufsize" was set above 1MB, because the block must first fit in the buffer. In that case, a client sending a large enough header is enough to hit the issue. So, perform the same tests than those performed when a header or a trailer is added, instead of corrupting the message. In addition, a BUG_ON() was added in htx_change_blk_value_len() and htx_set_blk_value_len() to catch any other caller missing the test. Reported by: Adam Crawford This patch must be backported to all stable versions. --- diff --git a/include/haproxy/htx.h b/include/haproxy/htx.h index 70f5f1f51..55b41838f 100644 --- a/include/haproxy/htx.h +++ b/include/haproxy/htx.h @@ -360,6 +360,7 @@ static inline void htx_change_blk_value_len(struct htx *htx, struct htx_blk *blk switch (type) { case HTX_BLK_HDR: case HTX_BLK_TLR: + BUG_ON(newlen > 1048575); oldlen = (blk->info >> 8) & 0xfffff; blk->info = (type << 28) + (newlen << 8) + (blk->info & 0xff); break; @@ -392,6 +393,7 @@ static inline void htx_set_blk_value_len(struct htx_blk *blk, uint32_t vlen) switch (type) { case HTX_BLK_HDR: case HTX_BLK_TLR: + BUG_ON(vlen > 1048575); blk->info = (type << 28) + (vlen << 8) + (blk->info & 0xff); break; case HTX_BLK_REQ_SL: diff --git a/reg-tests/http-rules/replace_header_limits.vtc b/reg-tests/http-rules/replace_header_limits.vtc new file mode 100644 index 000000000..5fd5a6c90 --- /dev/null +++ b/reg-tests/http-rules/replace_header_limits.vtc @@ -0,0 +1,77 @@ +varnishtest "replace-header/replace-value length limits" + +# In an HTX message, the value of a header or a trailer is limited to 1048575 +# bytes, because of the way its length is encoded in the block descriptor. This +# limit is tested when a header is added. This test checks it is also enforced +# when a header is updated. In this case, the rewrite must fail instead of +# silently corrupting the message. +# +# A "tune.bufsize" greater than 1MB is required to be able to exceed this limit. + +feature ignore_unknown_macro + +server s1 { + rxreq + expect req.url == /ok + expect req.http.x-test-len == 524288 + txresp +} -start + +haproxy h1 -conf { + global + tune.bufsize 2097152 + .if feature(THREAD) + thread-groups 1 + .endif + + defaults + mode http + timeout connect "${HAPROXY_TEST_TIMEOUT-5s}" + timeout client "${HAPROXY_TEST_TIMEOUT-5s}" + timeout server "${HAPROXY_TEST_TIMEOUT-5s}" + + frontend fe + bind "fd@${fe}" + + # Each rule below doubles the length of the x-test value. The client + # sends a 256 bytes long value, so these 11 rules lead to a 524288 + # bytes long value, just under the limit. + http-request replace-value x-test (.*) \1\1 + http-request replace-value x-test (.*) \1\1 + http-request replace-value x-test (.*) \1\1 + http-request replace-value x-test (.*) \1\1 + http-request replace-value x-test (.*) \1\1 + http-request replace-value x-test (.*) \1\1 + http-request replace-value x-test (.*) \1\1 + http-request replace-value x-test (.*) \1\1 + http-request replace-value x-test (.*) \1\1 + http-request replace-value x-test (.*) \1\1 + http-request replace-value x-test (.*) \1\1 + + # This last one would lead to a 1048576 bytes long value, just above + # the limit. The rewrite must fail. + http-request replace-value x-test (.*) \1\1 if { path /too-big } + + # Don't forward the huge header, only its length + http-request set-header x-test-len "%[req.hdr(x-test),length]" + http-request del-header x-test + + default_backend be + + backend be + server s1 ${s1_addr}:${s1_port} +} -start + +# 11 doublings: the value remains under the limit, the rewrites succeed +client c1 -connect ${h1_fe_sock} { + txreq -url /ok -hdr "x-test: 0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + rxresp + expect resp.status == 200 +} -run + +# 12 doublings: the value would exceed the limit, the last rewrite must fail +client c2 -connect ${h1_fe_sock} { + txreq -url /too-big -hdr "x-test: 0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef" + rxresp + expect resp.status == 500 +} -run diff --git a/src/htx.c b/src/htx.c index 5dcd433ad..100abab32 100644 --- a/src/htx.c +++ b/src/htx.c @@ -645,15 +645,24 @@ struct htx_blk *htx_add_data_atonce(struct htx *htx, struct ist data) struct htx_blk *htx_replace_blk_value(struct htx *htx, struct htx_blk *blk, const struct ist old, const struct ist new) { + enum htx_blk_type type; struct ist n, v; int32_t delta; int ret; + type = htx_get_blk_type(blk); n = htx_get_blk_name(htx, blk); v = htx_get_blk_value(htx, blk); delta = new.len - old.len; - if (unlikely(htx_get_blk_type(blk) < HTX_BLK_EOH && htx_hdrs_too_big(htx, delta))) + /* The value length of a header or a trailer is encoded on 20 bits. It + * must not overflow on the other fields of the block descriptor. + */ + if (unlikely((type == HTX_BLK_HDR || type == HTX_BLK_TLR) && + v.len + delta > 1048575)) + return NULL; + + if (unlikely(type < HTX_BLK_EOH && htx_hdrs_too_big(htx, delta))) return NULL; ret = htx_prepare_blk_expansion(htx, blk, delta); @@ -742,7 +751,7 @@ struct htx_blk *htx_replace_blk_value(struct htx *htx, struct htx_blk *blk, free_trash_chunk(chunk); } - if (htx_get_blk_type(blk) < HTX_BLK_EOH) + if (type < HTX_BLK_EOH) htx->hdrs_data += delta; return blk; @@ -920,6 +929,9 @@ struct htx_blk *htx_replace_header(struct htx *htx, struct htx_blk *blk, if (type != HTX_BLK_HDR) return NULL; + if (name.len > 255 || value.len > 1048575) + return NULL; + delta = name.len + value.len - htx_get_blksz(blk); if (unlikely(htx_hdrs_too_big(htx, delta))) return NULL;