BUG/MAJOR: hpack: don't return direct references to the dynamic headers table

Maximilian Böhm and Lucas Rolff both reported some random failed requests
with HTTP/2. Upon deep investigation on detailed traces provided by Lucas,
it turned out that some header names were occasionally corrupted and used
to point to random strings within the dynamic headers table.

The HPACK decoder must always return copies of header names that point
to the dynamic headers table. Otherwise, the insertion of a header after
the current one leading to a reorganization of the table will change the
data the pointer designates. Unfortunately, one such copy was missing for
indexed names, leading to random request failures due to invalid header
names.

Many thanks to Lucas who ran a large number of tests with full traces
helping to capture a reproduceable sequence exhibiting this issue.

This patch must be backported to 1.8.

diff --git a/src/hpack-dec.c b/src/hpack-dec.c
index 454f55cb7349996d4944d3327c435a198ded09bb..dfbcaff2776a5a492fd1c4c746fa9f1fff7ef9da 100644 (file)
--- a/src/hpack-dec.c
+++ b/src/hpack-dec.c
@@ -365,8 +365,13 @@ int hpack_decode_frame(struct hpack_dht *dht, const uint8_t *raw, uint32_t len,
                        if (!must_index)
                                name.len = hpack_idx_to_phdr(idx);
 
-                       if (!name.len)
-                               name = hpack_idx_to_name(dht, idx);
+                       if (!name.len) {
+                               name = hpack_alloc_string(tmp, idx, hpack_idx_to_name(dht, idx));
+                               if (!name.ptr) {
+                                       ret = -HPACK_ERR_TOO_LARGE;
+                                       goto leave;
+                               }
+                       }
                        /* <name> and <value> are correctly filled here */
                }