BUG/CRITICAL: hpack: fix improper sign check on the header index value

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.

diff --git a/include/common/hpack-tbl.h b/include/common/hpack-tbl.h
index ffa866bb5ae7b7070ddda34d29442bce7993eb2e..385f3860bbb27a8f45b9d30b56d44b3fca93ff33 100644 (file)
--- 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 <idx> is valid for table <dht> */
-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 16a722f99461f598deda6c098e758bc8290a6e17..148a9a21571629b07f4780161f79c67d5c512a10 100644 (file)
--- 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 02e5a5cfe76f3869c85b94613aa99955a6f393ae..24eb7c444a11fcb3db34cedd943e148289b1b89f 100644 (file)
--- 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];