From: Nataliia Lysychkina -X (nlysychk - SOFTSERVE INC at Cisco) Date: Fri, 29 Aug 2025 03:58:46 +0000 (+0000) Subject: Pull request #4878: protocols: add sanity checks for tcp and ipv4 options to prevent... X-Git-Tag: 3.9.5.0~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=16e5af5f91a67e4dc32bc4066f57a1487d126239;p=thirdparty%2Fsnort3.git Pull request #4878: protocols: add sanity checks for tcp and ipv4 options to prevent out-of-buffer access Merge in SNORT/snort3 from ~NLYSYCHK/snort3:tcp_options to master Squashed commit of the following: commit 3cd74355cb44339cc3e8ffe318ed3c90534f24f6 Author: Nataliia Lysychkina 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 a30e8f1bc..94ed4eae2 100644 --- 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 dac0921a9..c3e834305 100644 --- 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 4726e40a2..5b94906ee 100644 --- 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(len); + else if (len < 2) + { + static const IpOptions eol_opt = {IPOptionCodes::EOL, 0, {0}}; + return eol_opt; + } else return reinterpret_cast(data[len -2]); #endif diff --git a/src/protocols/tcp_options.cc b/src/protocols/tcp_options.cc index 7a9dbe909..9a2a91ac1 100644 --- 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 6c0b9c6f8..82556a346 100644 --- 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(len); + else if (len < 2) + { + static const TcpOption eol_opt = {TcpOptCode::EOL, 0, {0}}; + return eol_opt; + } else return reinterpret_cast(data[len -2]); }