fix: limit QPACK header list size during decoding (#3618)

* fix: limit QPACK header list size during decoding

A maliciously crafted header block with many indexed references to large
dynamic table entries could use lots of memory, as `decode_header_block` had
no bound on the number of headers it would accumulate. Cap the total
uncompressed header list size to `LiteralReader::MAX_LEN` using
a `checked_sub` budget, consistent with RFC 9113 §6.5.2.

Fixes #3616.

* Improve coverage

Author: larseggert

neqo-qpack/src/header_block.rs

@@ -22,8 +22,8 @@ use crate::{
         NO_PREFIX,
     },
     qpack_send_buf::Encoder as _,
-    reader::{ReceiverBufferWrapper, parse_utf8},
-    table::HeaderTable,
+    reader::{LiteralReader, ReceiverBufferWrapper, parse_utf8},
+    table::{ADDITIONAL_TABLE_ENTRY_SIZE, HeaderTable},
 };
 
 #[derive(Default, Debug, PartialEq, Eq)]
@@ -210,47 +210,42 @@ impl<'a> HeaderDecoder<'a> {
             return Ok(HeaderDecoderResult::Blocked(self.req_insert_cnt));
         }
         let mut h: Vec<Header> = Vec::new();
+        let mut remaining = LiteralReader::MAX_LEN;
 
         while !self.buf.done() {
             let b = Error::map_error(self.buf.peek(), Error::Decompression)?;
-            if HEADER_FIELD_INDEX_STATIC.cmp_prefix(b) {
-                h.push(Error::map_error(
-                    self.read_indexed_static(),
-                    Error::Decompression,
-                )?);
+            let header = if HEADER_FIELD_INDEX_STATIC.cmp_prefix(b) {
+                Error::map_error(self.read_indexed_static(), Error::Decompression)?
             } else if HEADER_FIELD_INDEX_DYNAMIC.cmp_prefix(b) {
-                h.push(Error::map_error(
-                    self.read_indexed_dynamic(table),
-                    Error::Decompression,
-                )?);
+                Error::map_error(self.read_indexed_dynamic(table), Error::Decompression)?
             } else if HEADER_FIELD_INDEX_DYNAMIC_POST.cmp_prefix(b) {
-                h.push(Error::map_error(
-                    self.read_indexed_dynamic_post(table),
-                    Error::Decompression,
-                )?);
+                Error::map_error(self.read_indexed_dynamic_post(table), Error::Decompression)?
             } else if HEADER_FIELD_LITERAL_NAME_REF_STATIC.cmp_prefix(b) {
-                h.push(Error::map_error(
+                Error::map_error(
                     self.read_literal_with_name_ref_static(),
                     Error::Decompression,
-                )?);
+                )?
             } else if HEADER_FIELD_LITERAL_NAME_REF_DYNAMIC.cmp_prefix(b) {
-                h.push(Error::map_error(
+                Error::map_error(
                     self.read_literal_with_name_ref_dynamic(table),
                     Error::Decompression,
-                )?);
+                )?
             } else if HEADER_FIELD_LITERAL_NAME_LITERAL.cmp_prefix(b) {
-                h.push(Error::map_error(
-                    self.read_literal_with_name_literal(),
-                    Error::Decompression,
-                )?);
+                Error::map_error(self.read_literal_with_name_literal(), Error::Decompression)?
             } else if HEADER_FIELD_LITERAL_NAME_REF_DYNAMIC_POST.cmp_prefix(b) {
-                h.push(Error::map_error(
+                Error::map_error(
                     self.read_literal_with_name_ref_dynamic_post(table),
                     Error::Decompression,
-                )?);
+                )?
             } else {
                 unreachable!("All prefixes are covered");
-            }
+            };
+            remaining = remaining
+                .checked_sub(
+                    header.name().len() + header.value().len() + ADDITIONAL_TABLE_ENTRY_SIZE,
+                )
+                .ok_or(Error::Decompression)?;
+            h.push(header);
         }
 
         qtrace!("[{self}] done decoding header block");
@@ -393,7 +388,10 @@ impl<'a> HeaderDecoder<'a> {
 #[cfg_attr(coverage_nightly, coverage(off))]
 mod tests {
 
-    use super::{HeaderDecoder, HeaderDecoderResult, HeaderEncoder, HeaderTable};
+    use super::{
+        ADDITIONAL_TABLE_ENTRY_SIZE, HeaderDecoder, HeaderDecoderResult, HeaderEncoder,
+        HeaderTable, LiteralReader,
+    };
     use crate::Error;
 
     const INDEX_STATIC_TEST: &[(u64, &[u8], &str, &str)] = &[
@@ -927,4 +925,49 @@ mod tests {
             decoder_h.decode_header_block(&table, 1000, 0).unwrap_err()
         );
     }
+
+    /// A header block that references the same static table entry many times must be
+    /// rejected before exhausting memory.
+    #[test]
+    fn header_list_size_limit() {
+        let table = HeaderTable::new(false);
+        // 0xC0 = HEADER_FIELD_INDEX_STATIC with index 0 (:authority, "").
+        let entry_size = ":authority".len() + ADDITIONAL_TABLE_ENTRY_SIZE;
+        let reps = LiteralReader::MAX_LEN / entry_size + 1;
+        // Prefix: required_insert_cnt=0, base_delta=0 (2 bytes: 0x00, 0x00)
+        let mut buf = vec![0x00u8, 0x00];
+        buf.resize(buf.len() + reps, 0xC0);
+        let mut decoder_h = HeaderDecoder::new(&buf);
+        assert_eq!(
+            Error::Decompression,
+            decoder_h.decode_header_block(&table, 1000, 0).unwrap_err()
+        );
+    }
+
+    /// Truncated input for each header-block prefix type must propagate as
+    /// `Error::Decompression`. Each byte sets the maximum value for its prefix's
+    /// integer field, forcing a continuation byte that is not present.
+    #[test]
+    fn decode_truncated_for_each_prefix() {
+        const TRUNCATED_PREFIXES: &[u8] = &[
+            0xFF, // HEADER_FIELD_INDEX_STATIC
+            0xBF, // HEADER_FIELD_INDEX_DYNAMIC
+            0x1F, // HEADER_FIELD_INDEX_DYNAMIC_POST
+            0x5F, // HEADER_FIELD_LITERAL_NAME_REF_STATIC
+            0x4F, // HEADER_FIELD_LITERAL_NAME_REF_DYNAMIC
+            0x3F, // HEADER_FIELD_LITERAL_NAME_LITERAL
+            0x07, // HEADER_FIELD_LITERAL_NAME_REF_DYNAMIC_POST
+        ];
+        let mut table = HeaderTable::new(false);
+        fill_table(&mut table);
+        for &prefix in TRUNCATED_PREFIXES {
+            let buf = [0x00, 0x00, prefix];
+            let mut decoder_h = HeaderDecoder::new(&buf);
+            assert_eq!(
+                Error::Decompression,
+                decoder_h.decode_header_block(&table, 1000, 0).unwrap_err(),
+                "prefix {prefix:#04x}"
+            );
+        }
+    }
 }

neqo-qpack/src/reader.rs

@@ -266,7 +266,7 @@ impl LiteralReader {
     /// The Gecko limit is in `network.http.max_response_header_size` and defaults to
     /// 393216 bytes (384 KB), see `modules/libpref/init/StaticPrefList.yaml`. We use
     /// the same limit.
-    const MAX_LEN: usize = 384 * 1024;
+    pub(crate) const MAX_LEN: usize = 384 * 1024;
 
     /// Creates `LiteralReader` with the first byte. This constructor is always used
     /// when a literal has a prefix.