From 7f2a44d319283b5d8e551640bb63c15a2df20358 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Mon, 17 Sep 2018 14:07:33 +0200 Subject: [PATCH] BUG/CRITICAL: hpack: fix improper sign check on the header index value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tim Düsterhus found using afl-fuzz that some parts of the HPACK decoder use incorrect bounds checking which do not catch negative values after a type cast. The first culprit is hpack_valid_idx() which takes a signed int and is fed with an unsigned one, but a few others are affected as well due to being designed to work with an uint16_t as in the table header, thus not being able to detect the high offset bits, though they are not exposed if hpack_valid_idx() is fixed. The impact is that the HPACK decoder can be crashed by an out-of-bounds read. The only work-around without this patch is to disable H2 in the configuration. CVE-2018-14645 was assigned to this bug. This patch addresses all of these issues at once. It must be backported to 1.8. --- include/common/hpack-tbl.h | 6 +++--- src/hpack-dec.c | 2 +- src/hpack-tbl.c | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/include/common/hpack-tbl.h b/include/common/hpack-tbl.h index ffa866bb5..385f3860b 100644 --- a/include/common/hpack-tbl.h +++ b/include/common/hpack-tbl.h @@ -155,7 +155,7 @@ static inline const struct hpack_dte *hpack_get_dte(const struct hpack_dht *dht, } /* returns non-zero if is valid for table */ -static inline int hpack_valid_idx(const struct hpack_dht *dht, uint16_t idx) +static inline int hpack_valid_idx(const struct hpack_dht *dht, uint32_t idx) { return idx < dht->used + HPACK_SHT_SIZE; } @@ -181,7 +181,7 @@ static inline struct ist hpack_get_value(const struct hpack_dht *dht, const stru } /* takes an idx, returns the associated name */ -static inline struct ist hpack_idx_to_name(const struct hpack_dht *dht, int idx) +static inline struct ist hpack_idx_to_name(const struct hpack_dht *dht, uint32_t idx) { const struct hpack_dte *dte; @@ -196,7 +196,7 @@ static inline struct ist hpack_idx_to_name(const struct hpack_dht *dht, int idx) } /* takes an idx, returns the associated value */ -static inline struct ist hpack_idx_to_value(const struct hpack_dht *dht, int idx) +static inline struct ist hpack_idx_to_value(const struct hpack_dht *dht, uint32_t idx) { const struct hpack_dte *dte; diff --git a/src/hpack-dec.c b/src/hpack-dec.c index 16a722f99..148a9a215 100644 --- a/src/hpack-dec.c +++ b/src/hpack-dec.c @@ -114,7 +114,7 @@ static inline int hpack_idx_to_phdr(uint32_t idx) * allocated there. In case of allocation failure, returns a string whose * pointer is NULL. */ -static inline struct ist hpack_alloc_string(struct buffer *store, int idx, +static inline struct ist hpack_alloc_string(struct buffer *store, uint32_t idx, struct ist in) { struct ist out; diff --git a/src/hpack-tbl.c b/src/hpack-tbl.c index 02e5a5cfe..24eb7c444 100644 --- a/src/hpack-tbl.c +++ b/src/hpack-tbl.c @@ -113,7 +113,7 @@ static inline unsigned int hpack_dht_get_tail(const struct hpack_dht *dht) /* dump the whole dynamic header table */ static void hpack_dht_dump(FILE *out, const struct hpack_dht *dht) { - int i; + unsigned int i; unsigned int slot; char name[4096], value[4096];