BUG/MINOR: qpack: fix 1-byte OOB read in qpack_decode_fs_pfx()

In qpack_decode_fs_pfx(), if the first qpack_get_varint() call
consumes the entire buffer, the code would perform a 1-byte
out-of-bounds read when accessing the sign bit via **raw.

This patch adds an explicit length check at the beginning of
qpack_get_varint(), which systematically secures all other callers
against empty inputs. It also adds a necessary check before the
second varint call in qpack_decode_fs_pfx() to ensure data is still
available before dereferencing the pointer to extract the sign bit,
returning QPACK_RET_TRUNCATED if the buffer is exhausted.

Thank you to Kamil Frankowicz for having reported this.

Must be backported as far as 2.6.

diff --git a/src/qpack-dec.c b/src/qpack-dec.c
index 130815d6a8a63ba883d27de7103f18805b8310cb..d1afcd6b1a6cfc26f66a6393bd31a0278073e9fb 100644 (file)
--- a/src/qpack-dec.c
+++ b/src/qpack-dec.c
@@ -65,6 +65,9 @@ static uint64_t qpack_get_varint(const unsigned char **buf, uint64_t *len_in, in
        const uint8_t *raw = *buf;
        uint8_t shift = 0;
 
+       if (len == 0)
+               goto too_short;
+
        len--;
        ret = *raw++ & ((1ULL << b) - 1);
        if (ret != (uint64_t)((1ULL << b) - 1))
@@ -220,6 +223,13 @@ static int qpack_decode_fs_pfx(uint64_t *enc_ric, uint64_t *db, int *sign_bit,
        if (*len == (uint64_t)-1)
                return -QPACK_RET_RIC;
 
+       /* Ensure at least one byte remains for the sign bit
+        * and the start of the Delta Base varint.
+        */
+       if (!*len)
+               return -QPACK_RET_TRUNCATED;
+
+       /* Safe access to the sign bit thanks to the check above */
        *sign_bit = **raw & 0x8;
        *db = qpack_get_varint(raw, len, 7);
        if (*len == (uint64_t)-1)