Pull request #4878: protocols: add sanity checks for tcp and ipv4 options to prevent...

Merge in SNORT/snort3 from ~NLYSYCHK/snort3:tcp_options to master

Squashed commit of the following:

commit 3cd74355cb44339cc3e8ffe318ed3c90534f24f6
Author: Nataliia Lysychkina <nlysychk@cisco.com>
Date:   Thu Aug 21 17:26:23 2025 +0530

    protocols: add sanity checks for tcp and ipv4 options to prevent out-of-buffer access

diff --git a/src/network_inspectors/rna/rna_fingerprint_tcp.cc b/src/network_inspectors/rna/rna_fingerprint_tcp.cc
index a30e8f1bca84c43979dba3651670fef8ad5725a5..94ed4eae2fe35cfbcd41cc8e120839a7405fa44b 100644 (file)
--- a/src/network_inspectors/rna/rna_fingerprint_tcp.cc
+++ b/src/network_inspectors/rna/rna_fingerprint_tcp.cc
@@ -836,11 +836,97 @@ TEST_CASE("get_tcp_option", "[rna_fingerprint_tcp]")
     int pos;
 
     // Check the default case when the desired option does not match
-    CHECK( -1 == get_tcp_option(&p, tcp::TcpOptCode::EOL, pos) );
+    CHECK(( -1 == get_tcp_option(&p, tcp::TcpOptCode::EOL, pos) && pos == -1 ));
 
-    // Check the case when NOP option is matched
-    cooked_pkt[54] = 1; // 34 + TCP_MIN_HEADER_LEN = 54
-    CHECK( get_tcp_option(&p, tcp::TcpOptCode::NOP, pos) == 1 );
+    p.layers = saved_layers;
+}
+
+TEST_CASE("get_tcp_option_NOP", "[rna_fingerprint_tcp]")
+{
+    // Hex buffer from a packet with NOP option
+    uint8_t cooked_pkt[] = "\x00\x21\x91\x01\xb2\x48\xaa\x00\x04\x00\x0a\x04\x08\x00" // Ethernet Header
+                       "\x45\x00\x00\x2c\x00\x01\x00\x00\x40\x06\x82\xa8" // IP Header
+                       "\xc0\xa8\x00\x59\x48\xa7\xe8\x90" // IP Source and Destination Addresses
+                       "\x23\x34\x00\x50" // TCP Source Port, Destination Port
+                       "\x00\x00\x23\x5a\x00\x00\x00\x00" // TCP Sequence Number, Acknowledgement Number
+                       "\x60\x02" // TCP Data Offset, Flags (SYN)
+                       "\x20\x00" // TCP Window Size
+                       "\x17\xf8" // TCP Checksum
+                       "\x00\x00" // TCP Urgent Pointer
+                       "\x01\x01\x01\x01"; // TCP Options: NOPs (0x01)
+    Packet p(false);
+    p.pkt = cooked_pkt;
+    p.ptrs.tcph = ( const tcp::TCPHdr* )( cooked_pkt + 34 );
+    p.num_layers = 1;
+    Layer cooked_layer;
+    cooked_layer.start = cooked_pkt + 34;
+    cooked_layer.length = 24; // TCP_MIN_HEADER_LEN + 4
+    auto saved_layers = p.layers;
+    p.layers = &cooked_layer;
+    int pos;
+
+    CHECK((get_tcp_option(&p, tcp::TcpOptCode::NOP, pos) == 1 && pos == 0));
+    CHECK((get_tcp_option(&p, tcp::TcpOptCode::TIMESTAMP, pos) == -1 && pos == -1));
+
+    p.layers = saved_layers;
+}
+
+TEST_CASE("get_tcp_option_bad_layers", "[rna_fingerprint_tcp]")
+{
+    // Hex buffer from a packet without options and 4 bytes of payload after header
+    uint8_t cooked_pkt[] = "\x00\x21\x91\x01\xb2\x48\xaa\x00\x04\x00\x0a\x04\x08\x00" // Ethernet Header
+                       "\x45\x00\x00\x28\x00\x01\x00\x00\x40\x06\x88\x96" // IP Header
+                       "\xc0\xa8\x00\x59\x48\xa7\xe8\x90" // IP Source and Destination Addresses
+                       "\x23\x34\x00\x50" // TCP Source Port, Destination Port
+                       "\x00\x00\x23\x5a\x00\x00\x00\x00" // TCP Sequence Number, Acknowledgement Number
+                       "\x50\x02" // TCP Data Offset, Flags (SYN)
+                       "\x20\x00" // TCP Window Size
+                       "\x56\xcb" // TCP Checksum
+                       "\x00\x00" // TCP Urgent Pointer
+                       "\x05\x00\x0e\x07";
+    Packet p(false);
+    p.pkt = cooked_pkt;
+    p.ptrs.tcph = (const tcp::TCPHdr*)(cooked_pkt + 34);
+    p.num_layers = 1;
+    Layer cooked_layer;
+    cooked_layer.start = cooked_pkt + 34;
+    cooked_layer.length = 32; // Incorrect length in layer
+    auto saved_layers = p.layers;
+    p.layers = &cooked_layer;
+    int pos;
+
+    CHECK((get_tcp_option(&p, tcp::TcpOptCode::NOP, pos) == -1 && pos == -1));
+    CHECK((get_tcp_option(&p, tcp::TcpOptCode::SACK, pos) == -1 && pos == -1));
+
+    p.layers = saved_layers;
+}
+
+TEST_CASE("get_tcp_option_malformed", "[rna_fingerprint_tcp]")
+{
+    // Hex buffer from a packet with 1 malformed option of type SACKOK
+    uint8_t cooked_pkt[] = "\x00\x21\x91\x01\xb2\x48\xaa\x00\x04\x00\x0a\x04\x08\x00" // Ethernet Header
+                       "\x45\x00\x00\x2c\x00\x01\x00\x00\x40\x06\x88\x92" // IP Header
+                       "\xc0\xa8\x00\x59\x48\xa7\xe8\x90" // IP Source and Destination Addresses
+                       "\x23\x34\x00\x50" // TCP Source Port, Destination Port
+                       "\x00\x00\x23\x5a\x00\x00\x00\x00" // TCP Sequence Number, Acknowledgement Number
+                       "\x60\x02" // TCP Data Offset, Flags (SYN)
+                       "\x20\x00" // TCP Window Size
+                       "\x41\xc3" // TCP Checksum
+                       "\x00\x00" // TCP Urgent Pointer
+                       "\x04\x00\x0e\x07"; // malformed SACKOK option, length 0
+    Packet p(false);
+    p.pkt = cooked_pkt;
+    p.ptrs.tcph = (const tcp::TCPHdr*)(cooked_pkt + 34);
+    p.num_layers = 1;
+    Layer cooked_layer;
+    cooked_layer.start = cooked_pkt + 34;
+    cooked_layer.length = 24;
+    auto saved_layers = p.layers;
+    p.layers = &cooked_layer;
+    int pos;
+
+    CHECK((get_tcp_option(&p, tcp::TcpOptCode::SACKOK, pos) == 1 && pos == 0));
+    CHECK((get_tcp_option(&p, tcp::TcpOptCode::NOP, pos) == -1 && pos == -1));
 
     p.layers = saved_layers;
 }

diff --git a/src/protocols/ipv4_options.cc b/src/protocols/ipv4_options.cc
index dac0921a9719b38469bfaef6d69868215016b752..c3e8343057e20798b6d1e2b16aebb4e69bcef890 100644 (file)
--- a/src/protocols/ipv4_options.cc
+++ b/src/protocols/ipv4_options.cc
@@ -41,6 +41,9 @@ IpOptionIterator::IpOptionIterator(const IP4Hdr* const ip4_header, const Packet*
     start_ptr = hdr + IP4_HEADER_LEN;
     end_ptr = start_ptr;
 
+    if (!ip4_header->has_options())
+        return;
+
     for (int i = p->num_layers-1; i >= 0; --i)
     {
         if (p->layers[i].start == (const uint8_t*)ip4_header)

diff --git a/src/protocols/ipv4_options.h b/src/protocols/ipv4_options.h
index 4726e40a26cbd93a6cddd27af8ec6a0d6169c011..5b94906eeccc064663465f7c2d8247dc9d29fce1 100644 (file)
--- a/src/protocols/ipv4_options.h
+++ b/src/protocols/ipv4_options.h
@@ -73,6 +73,11 @@ struct IpOptions
         // ... and the legible code
         if ( (uint8_t)code <= 1 )
             return reinterpret_cast<const IpOptions&>(len);
+        else if (len < 2)
+        {
+            static const IpOptions eol_opt = {IPOptionCodes::EOL, 0, {0}};
+            return eol_opt;
+        }
         else
             return reinterpret_cast<const IpOptions&>(data[len -2]);
 #endif

diff --git a/src/protocols/tcp_options.cc b/src/protocols/tcp_options.cc
index 7a9dbe90940fda886e3b65041aa64538e1d75d69..9a2a91ac13c51d3889ec57d58a4916a2c08fd8be 100644 (file)
--- a/src/protocols/tcp_options.cc
+++ b/src/protocols/tcp_options.cc
@@ -54,6 +54,9 @@ TcpOptIterator::TcpOptIterator(const TCPHdr* const tcp_header, const Packet* con
     start_ptr = hdr + TCP_MIN_HEADER_LEN;
     end_ptr = start_ptr; // == begin()
 
+    if (!tcp_header->has_options())
+        return;
+
     for (int i = p->num_layers-1; i >= 0; --i)
     {
         if (p->layers[i].start == (const uint8_t*)tcp_header)

diff --git a/src/protocols/tcp_options.h b/src/protocols/tcp_options.h
index 6c0b9c6f88f7ca103862798e41357a3bd31c9fb9..82556a34657306e96e37f6383d66cf43cad9b618 100644 (file)
--- a/src/protocols/tcp_options.h
+++ b/src/protocols/tcp_options.h
@@ -111,6 +111,11 @@ struct TcpOption
     {
         if ( (uint8_t)code <= 1 )
             return reinterpret_cast<const TcpOption&>(len);
+        else if (len < 2)
+        {
+            static const TcpOption eol_opt = {TcpOptCode::EOL, 0, {0}};
+            return eol_opt;
+        }
         else
             return reinterpret_cast<const TcpOption&>(data[len -2]);
     }