upstream/haproxy
1
0
Fork 0
mirror of https://github.com/haproxy/haproxy.git synced 2026-03-06 15:20:56 -05:00
Code Issues Projects Releases Packages Wiki Activity Actions 8

BUG/MEDIUM: hpack: correctly deal with too large decoded numbers

Browse source 
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.
This commit is contained in:
Willy Tarreau 2026-03-04 11:37:12 +01:00
parent b1441c6440
commit 7315428615
1 changed files with 16 additions and 11 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

27
src/hpack-dec.c
View file

@ -50,13 +50,15 @@
/* reads a varint from <raw>'s lowest <b> bits and <len> 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 <len_in> 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;