CVE-2024-32663-1

commit 08d93f7c3762781b743f88f9fdc4389eb9c3eb64
Author: Philippe Antoine <pantoine@oisf.net>
Date:   Wed Mar 27 14:33:54 2024 +0100

    http2: use a reference counter for headers

    Ticket: 6892

    As HTTP hpack header compression allows one single byte to
    express a previously seen arbitrary-size header block (name+value)
    we should avoid to copy the vectors data, but just point
    to the same data, while reamining memory safe, even in the case
    of later headers eviction from the dybnamic table.

    Rust std solution is Rc, and the use of clone, so long as the
    data is accessed by only one thread.

    (cherry picked from commit 390f09692eb99809c679d3f350c7cc185d163e1a)

Gbp-Pq: Name CVE-2024-32663-1.patch

diff --git a/rust/src/http2/detect.rs b/rust/src/http2/detect.rs
index 59180263231e2c7cccd07fe85e5346cb0680cc3b..540d4227c5a168655b71902db3af992968a6be83 100644 (file)
--- a/rust/src/http2/detect.rs
+++ b/rust/src/http2/detect.rs
@@ -23,6 +23,7 @@ use crate::core::STREAM_TOSERVER;
 use std::ffi::CStr;
 use std::mem::transmute;
 use std::str::FromStr;
+use std::rc::Rc;
 
 fn http2_tx_has_frametype(
     tx: &mut HTTP2Transaction, direction: u8, value: u8,
@@ -557,8 +558,8 @@ fn http2_tx_set_header(state: &mut HTTP2State, name: &[u8], input: &[u8]) {
     };
     let mut blocks = Vec::new();
     let b = parser::HTTP2FrameHeaderBlock {
-        name: name.to_vec(),
-        value: input.to_vec(),
+        name: Rc::new(name.to_vec()),
+        value: Rc::new(input.to_vec()),
         error: parser::HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeSuccess,
         sizeupdate: 0,
     };

diff --git a/rust/src/http2/parser.rs b/rust/src/http2/parser.rs
index 61e4557f6a5909f05503f6740117050d28501bdd..036ebd2f8e4419f7f809f4bf71d68c8349ebe031 100644 (file)
--- a/rust/src/http2/parser.rs
+++ b/rust/src/http2/parser.rs
@@ -25,6 +25,7 @@ use nom::Err;
 use nom::IResult;
 use std::fmt;
 use std::str::FromStr;
+use std::rc::Rc;
 
 #[repr(u8)]
 #[derive(Clone, Copy, PartialEq, FromPrimitive, Debug)]
@@ -281,8 +282,8 @@ fn http2_frame_header_static(n: u64, dyn_headers: &HTTP2DynTable) -> Option<HTTP
     };
     if name.len() > 0 {
         return Some(HTTP2FrameHeaderBlock {
-            name: name.as_bytes().to_vec(),
-            value: value.as_bytes().to_vec(),
+            name: Rc::new(name.as_bytes().to_vec()),
+            value: Rc::new(value.as_bytes().to_vec()),
             error: HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeSuccess,
             sizeupdate: 0,
         });
@@ -290,23 +291,23 @@ fn http2_frame_header_static(n: u64, dyn_headers: &HTTP2DynTable) -> Option<HTTP
         //use dynamic table
         if n == 0 {
             return Some(HTTP2FrameHeaderBlock {
-                name: Vec::new(),
-                value: Vec::new(),
+                name: Rc::new(Vec::new()),
+                value: Rc::new(Vec::new()),
                 error: HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeIndex0,
                 sizeupdate: 0,
             });
         } else if dyn_headers.table.len() + HTTP2_STATIC_HEADERS_NUMBER < n as usize {
             return Some(HTTP2FrameHeaderBlock {
-                name: Vec::new(),
-                value: Vec::new(),
+                name: Rc::new(Vec::new()),
+                value: Rc::new(Vec::new()),
                 error: HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeNotIndexed,
                 sizeupdate: 0,
             });
         } else {
             let indyn = dyn_headers.table.len() - (n as usize - HTTP2_STATIC_HEADERS_NUMBER);
             let headcopy = HTTP2FrameHeaderBlock {
-                name: dyn_headers.table[indyn].name.to_vec(),
-                value: dyn_headers.table[indyn].value.to_vec(),
+                name: dyn_headers.table[indyn].name.clone(),
+                value: dyn_headers.table[indyn].value.clone(),
                 error: HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeSuccess,
                 sizeupdate: 0,
             };
@@ -334,8 +335,10 @@ impl fmt::Display for HTTP2HeaderDecodeStatus {
 
 #[derive(Clone, Debug)]
 pub struct HTTP2FrameHeaderBlock {
-    pub name: Vec<u8>,
-    pub value: Vec<u8>,
+    // Use Rc reference counted so that indexed headers do not get copied.
+    // Otherwise, this leads to quadratic complexity in memory occupation.
+    pub name: Rc<Vec<u8>>,
+    pub value: Rc<Vec<u8>>,
     pub error: HTTP2HeaderDecodeStatus,
     pub sizeupdate: u64,
 }
@@ -386,7 +389,7 @@ fn http2_parse_headers_block_literal_common<'a>(
 ) -> IResult<&'a [u8], HTTP2FrameHeaderBlock> {
     let (i3, name, error) = if index == 0 {
         match http2_parse_headers_block_string(input) {
-            Ok((r, n)) => Ok((r, n, HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeSuccess)),
+            Ok((r, n)) => Ok((r, Rc::new(n), HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeSuccess)),
             Err(e) => Err(e),
         }
     } else {
@@ -398,7 +401,7 @@ fn http2_parse_headers_block_literal_common<'a>(
             )),
             None => Ok((
                 input,
-                Vec::new(),
+                Rc::new(Vec::new()),
                 HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeNotIndexed,
             )),
         }
@@ -408,7 +411,7 @@ fn http2_parse_headers_block_literal_common<'a>(
         i4,
         HTTP2FrameHeaderBlock {
             name,
-            value,
+            value: Rc::new(value),
             error,
             sizeupdate: 0,
         },
@@ -436,8 +439,8 @@ fn http2_parse_headers_block_literal_incindex<'a>(
     match r {
         Ok((r, head)) => {
             let headcopy = HTTP2FrameHeaderBlock {
-                name: head.name.to_vec(),
-                value: head.value.to_vec(),
+                name: head.name.clone(),
+                value: head.value.clone(),
                 error: head.error,
                 sizeupdate: 0,
             };
@@ -554,8 +557,8 @@ fn http2_parse_headers_block_dynamic_size<'a>(
         return Ok((
             i3,
             HTTP2FrameHeaderBlock {
-                name: Vec::new(),
-                value: Vec::new(),
+                name: Vec::new().into(),
+                value: Vec::new().into(),
                 error: HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeIntegerOverflow,
                 sizeupdate: 0,
             },
@@ -573,8 +576,8 @@ fn http2_parse_headers_block_dynamic_size<'a>(
     return Ok((
         i3,
         HTTP2FrameHeaderBlock {
-            name: Vec::new(),
-            value: Vec::new(),
+            name: Rc::new(Vec::new()),
+            value: Rc::new(Vec::new()),
             error: HTTP2HeaderDecodeStatus::HTTP2HeaderDecodeSizeUpdate,
             sizeupdate: maxsize2,
         },
@@ -928,8 +931,8 @@ mod tests {
         match r0 {
             Ok((remainder, hd)) => {
                 // Check the first message.
-                assert_eq!(hd.name, ":method".as_bytes().to_vec());
-                assert_eq!(hd.value, "GET".as_bytes().to_vec());
+                assert_eq!(hd.name, ":method".as_bytes().to_vec().into());
+                assert_eq!(hd.value, "GET".as_bytes().to_vec().into());
                 // And we should have no bytes left.
                 assert_eq!(remainder.len(), 0);
             }
@@ -945,8 +948,8 @@ mod tests {
         match r1 {
             Ok((remainder, hd)) => {
                 // Check the first message.
-                assert_eq!(hd.name, "accept".as_bytes().to_vec());
-                assert_eq!(hd.value, "*/*".as_bytes().to_vec());
+                assert_eq!(hd.name, "accept".as_bytes().to_vec().into());
+                assert_eq!(hd.value, "*/*".as_bytes().to_vec().into());
                 // And we should have no bytes left.
                 assert_eq!(remainder.len(), 0);
                 assert_eq!(dynh.table.len(), 1);
@@ -965,8 +968,8 @@ mod tests {
         match result {
             Ok((remainder, hd)) => {
                 // Check the first message.
-                assert_eq!(hd.name, ":authority".as_bytes().to_vec());
-                assert_eq!(hd.value, "localhost:3000".as_bytes().to_vec());
+                assert_eq!(hd.name, ":authority".as_bytes().to_vec().into());
+                assert_eq!(hd.value, "localhost:3000".as_bytes().to_vec().into());
                 // And we should have no bytes left.
                 assert_eq!(remainder.len(), 0);
                 assert_eq!(dynh.table.len(), 2);
@@ -983,8 +986,8 @@ mod tests {
         match r3 {
             Ok((remainder, hd)) => {
                 // same as before
-                assert_eq!(hd.name, ":authority".as_bytes().to_vec());
-                assert_eq!(hd.value, "localhost:3000".as_bytes().to_vec());
+                assert_eq!(hd.name, ":authority".as_bytes().to_vec().into());
+                assert_eq!(hd.value, "localhost:3000".as_bytes().to_vec().into());
                 // And we should have no bytes left.
                 assert_eq!(remainder.len(), 0);
                 assert_eq!(dynh.table.len(), 2);
@@ -1019,8 +1022,8 @@ mod tests {
         match r2 {
             Ok((remainder, hd)) => {
                 // Check the first message.
-                assert_eq!(hd.name, ":path".as_bytes().to_vec());
-                assert_eq!(hd.value, "/doc/manual/html/index.html".as_bytes().to_vec());
+                assert_eq!(hd.name, ":path".as_bytes().to_vec().into());
+                assert_eq!(hd.value, "/doc/manual/html/index.html".as_bytes().to_vec().into());
                 // And we should have no bytes left.
                 assert_eq!(remainder.len(), 0);
                 assert_eq!(dynh.table.len(), 2);