52 lines
2.5 KiB
Diff
52 lines
2.5 KiB
Diff
|
From 4e372dc350be5c72b88546bf03392a5793cea179 Mon Sep 17 00:00:00 2001
|
||
|
From: Willy Tarreau <w@1wt.eu>
|
||
|
Date: Sun, 29 Mar 2020 08:53:31 +0200
|
||
|
Subject: BUG/CRITICAL: hpack: never index a header into the headroom after
|
||
|
wrapping
|
||
|
|
||
|
The HPACK header table is implemented as a wrapping list inside a contigous
|
||
|
area. Headers names and values are stored from right to left while indexes
|
||
|
are stored from left to right. When there's no more room to store a new one,
|
||
|
we wrap to the right again, or possibly defragment it if needed. The condition
|
||
|
do use the right part (called tailroom) or the left part (called headroom)
|
||
|
depends on the location of the last inserted header. After wrapping happens,
|
||
|
the code forces to stick to tailroom by pretending there's no more headroom,
|
||
|
so that the size fit test always fails. The problem is that nothing prevents
|
||
|
from storing a header with an empty name and empty value, resulting in a
|
||
|
total size of zero bytes, which satisfies the condition to use the headroom.
|
||
|
Doing this in a wrapped buffer results in changing the "front" header index
|
||
|
and causing miscalculations on the available size and the addresses of the
|
||
|
next headers. This may even allow to overwrite some parts of the index,
|
||
|
opening the possibility to perform arbitrary writes into a 32-bit relative
|
||
|
address space.
|
||
|
|
||
|
This patch fixes the issue by making sure the headroom is considered only
|
||
|
when the buffer does not wrap, instead of relying on the zero size. This
|
||
|
must be backported to all versions supporting H2, which is as far as 1.8.
|
||
|
|
||
|
Many thanks to Felix Wilhelm of Google Project Zero for responsibly
|
||
|
reporting this problem with a reproducer and a detailed analysis.
|
||
|
---
|
||
|
src/hpack-tbl.c | 4 ++--
|
||
|
1 file changed, 2 insertions(+), 2 deletions(-)
|
||
|
|
||
|
diff --git a/src/hpack-tbl.c b/src/hpack-tbl.c
|
||
|
index 70d7f35834..727ff7a17b 100644
|
||
|
--- a/src/hpack-tbl.c
|
||
|
+++ b/src/hpack-tbl.c
|
||
|
@@ -346,9 +346,9 @@ int hpack_dht_insert(struct hpack_dht *dht, struct ist name, struct ist value)
|
||
|
* room left in the tail to suit the protocol, but tests show that in
|
||
|
* practice it almost never happens in other situations so the extra
|
||
|
* test is useless and we simply fill the headroom as long as it's
|
||
|
- * available.
|
||
|
+ * available and we don't wrap.
|
||
|
*/
|
||
|
- if (headroom >= name.len + value.len) {
|
||
|
+ if (prev == dht->front && headroom >= name.len + value.len) {
|
||
|
/* install upfront and update ->front */
|
||
|
dht->dte[head].addr = dht->dte[dht->front].addr - (name.len + value.len);
|
||
|
dht->front = head;
|
||
|
--
|
||
|
2.20.1
|
||
|
|