From 731542861533ac2cb1d83cfeb366754e48cc7919 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Wed, 4 Mar 2026 11:37:12 +0100 Subject: [PATCH] BUG/MEDIUM: hpack: correctly deal with too large decoded numbers The varint hpack decoder supports unbounded numbers but returns 32-bit results. This means that possible truncation my happen on some field lengths or indexes that would be emitted as quantities that do not fit in a 32-bit number. The final value will also depend on how the left shift operation behaves on the target architecture (e.g. whether bits are lost or used modulo 31). This could lead to a desynchronization of the HPACK stream decoding compared to what an external observer would see (e.g. from a network traffic capture). However, there isn't any impact between streams, HPACK is performed at the connection level, not at the stream level, so no stream may try to leverage this limitation to have any effect on another one. For the fix, instead of adding checks everywhere in the loop and for the final stage, let's rewrite the decoder to compare the read value to a max value that is shifted by 7 bits for every 7 bits read. This allows a sender to continue to emit zeroes for higher bits without being blocked, while detecting that a received value would overflow. The loop is now simpler as it deals both with values with the higher bit set and the final ones, and stops once the final value was recorded. A test on non-zero before performing the shift was added to please ubsan, though in practice zero shifted by any quantity remains zero. But the test is cheap so that's OK. Thanks to Guillaume Meunier, Head of Vulnerability Operations Center France at Orange Cyberdefense, for reporting this bug. This should be backported to all stable versions. --- src/hpack-dec.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/hpack-dec.c b/src/hpack-dec.c index 052a7c3da..e9a051007 100644 --- a/src/hpack-dec.c +++ b/src/hpack-dec.c @@ -50,13 +50,15 @@ /* reads a varint from 's lowest bits and bytes max (raw included). * returns the 32-bit value on success after updating raw_in and len_in. Forces - * len_in to (uint32_t)-1 on truncated input. + * len_in to (uint32_t)-1 on truncated input. The caller is responsible for + * providing a non-zero on input. */ static uint32_t get_var_int(const uint8_t **raw_in, uint32_t *len_in, int b) { uint32_t ret = 0; int len = *len_in; const uint8_t *raw = *raw_in; + uint32_t v, max = ~0; uint8_t shift = 0; len--; @@ -64,23 +66,26 @@ static uint32_t get_var_int(const uint8_t **raw_in, uint32_t *len_in, int b) if (ret != (uint32_t)((1 << b) - 1)) goto end; - while (len && (*raw & 128)) { - ret += ((uint32_t)(*raw++) & 127) << shift; - shift += 7; + do { + if (!len) + goto too_short; + v = *raw++; len--; - } - - /* last 7 bits */ - if (!len) - goto too_short; - len--; - ret += ((uint32_t)(*raw++) & 127) << shift; + if (v & 127) { // make UBSan happy + if ((v & 127) > max) + goto too_large; + ret += (v & 127) << shift; + } + max >>= 7; + shift += 7; + } while (v & 128); end: *raw_in = raw; *len_in = len; return ret; + too_large: too_short: *len_in = (uint32_t)-1; return 0;