From: Tom Peters (thopeter) <thopeter@cisco.com>
Date: Tue, 30 Mar 2021 19:53:18 +0000 (+0000)
Subject: Merge pull request #2817 in SNORT/snort3 from ~KATHARVE/snort3:h2i_hpack_fix to master
X-Git-Tag: 3.1.4.0~34
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a52561878cf3a9e8ca3fd8a3b0ba2169ec74fd82;p=thirdparty%2Fsnort3.git

Merge pull request #2817 in SNORT/snort3 from ~KATHARVE/snort3:h2i_hpack_fix to master

Squashed commit of the following:

commit baf855dbcfe551ac5a42bec110adf53a958b281f
Author: Katura Harvey <katharve@cisco.com>
Date:   Fri Mar 26 14:28:53 2021 -0400

    http2_inspect: fix possible read-after-free in hpack decoder
---

diff --git a/src/service_inspectors/http2_inspect/http2_hpack.cc b/src/service_inspectors/http2_inspect/http2_hpack.cc
index 3536f4c4f..9ab8fbe90 100644
--- a/src/service_inspectors/http2_inspect/http2_hpack.cc
+++ b/src/service_inspectors/http2_inspect/http2_hpack.cc
@@ -80,9 +80,8 @@ bool Http2HpackDecoder::decode_string_literal(const uint8_t* encoded_header_buff
 
 const HpackTableEntry* Http2HpackDecoder::get_hpack_table_entry(
     const uint8_t* encoded_header_buffer, const uint32_t encoded_header_length,
-    const Http2HpackIntDecode& decode_int, uint32_t& bytes_consumed)
+    const Http2HpackIntDecode& decode_int, uint32_t& bytes_consumed, uint64_t &index)
 {
-    uint64_t index;
     bytes_consumed = 0;
 
     if (!decode_int.translate(encoded_header_buffer, encoded_header_length, bytes_consumed,
@@ -102,12 +101,13 @@ const HpackTableEntry* Http2HpackDecoder::get_hpack_table_entry(
 bool Http2HpackDecoder::decode_indexed_name(const uint8_t* encoded_header_buffer,
     const uint32_t encoded_header_length, const Http2HpackIntDecode& decode_int,
     uint32_t& bytes_consumed, uint8_t* decoded_header_buffer, const uint32_t decoded_header_length,
-    uint32_t& bytes_written, Field& name)
+    uint32_t& bytes_written, Field& name, bool with_indexing)
 {
     bytes_written = bytes_consumed = 0;
 
+    uint64_t index;
     const HpackTableEntry* entry = get_hpack_table_entry(encoded_header_buffer,
-        encoded_header_length, decode_int, bytes_consumed);
+        encoded_header_length, decode_int, bytes_consumed, index);
     if (!entry)
         return false;
 
@@ -117,7 +117,16 @@ bool Http2HpackDecoder::decode_indexed_name(const uint8_t* encoded_header_buffer
         return false;
     }
 
-    name.set(entry->name);
+    // If this header will be added to the dynamic table and was from the dynamic table, we need to
+    // make a copy because the original table entry may be pruned and freed
+    if (with_indexing and index > HpackIndexTable::STATIC_MAX_INDEX)
+    {
+        uint8_t* name_copy = new uint8_t[entry->name.length()];
+        memcpy(name_copy, entry->name.start(), entry->name.length());
+        name.set(entry->name.length(), name_copy, true);
+    }
+    else
+        name.set(entry->name);
     return true;
 }
 
@@ -135,7 +144,7 @@ bool Http2HpackDecoder::decode_literal_header_line(const uint8_t* encoded_header
     {
         if (!decode_indexed_name(encoded_header_buffer, encoded_header_length, decode_int,
                 partial_bytes_consumed, decoded_header_buffer, decoded_header_length,
-                partial_bytes_written, name))
+                partial_bytes_written, name, with_indexing))
             return false;
     }
 
@@ -204,8 +213,9 @@ bool Http2HpackDecoder::decode_indexed_header(const uint8_t* encoded_header_buff
     uint32_t partial_bytes_written = 0;
     bytes_written = bytes_consumed = 0;
 
+    uint64_t index;
     const HpackTableEntry* const entry = get_hpack_table_entry(encoded_header_buffer,
-        encoded_header_length, decode_int, bytes_consumed);
+        encoded_header_length, decode_int, bytes_consumed, index);
     if (!entry)
         return false;
     name.set(entry->name);
diff --git a/src/service_inspectors/http2_inspect/http2_hpack.h b/src/service_inspectors/http2_inspect/http2_hpack.h
index d4fc5348e..d545eb41c 100644
--- a/src/service_inspectors/http2_inspect/http2_hpack.h
+++ b/src/service_inspectors/http2_inspect/http2_hpack.h
@@ -57,14 +57,14 @@ public:
         const uint32_t encoded_header_length, uint32_t& bytes_consumed);
     const HpackTableEntry* get_hpack_table_entry(const uint8_t* encoded_header_buffer,
         const uint32_t encoded_header_length, const Http2HpackIntDecode& decode_int,
-        uint32_t& bytes_consumed);
+        uint32_t& bytes_consumed, uint64_t &index);
     bool write_header_part(const Field& header, const uint8_t* suffix, uint32_t suffix_length,
         uint8_t* decoded_header_buffer, const uint32_t decoded_header_length,
         uint32_t& bytes_written);
     bool decode_indexed_name(const uint8_t* encoded_header_buffer,
         const uint32_t encoded_header_length, const Http2HpackIntDecode& decode_int,
         uint32_t& bytes_consumed, uint8_t* decoded_header_buffer, const uint32_t decoded_header_length,
-        uint32_t& bytes_written, Field& name);
+        uint32_t& bytes_written, Field& name, bool with_indexing);
     bool decode_literal_header_line(const uint8_t* encoded_header_buffer,
         const uint32_t encoded_header_length, const uint8_t name_index_mask,
         const Http2HpackIntDecode& decode_int, bool with_indexing, uint32_t& bytes_consumed,