Merge pull request #3085 in SNORT/snort3 from ~SMINUT/snort3:tcp_opt_iter to master

Squashed commit of the following:

commit 218596bc8e12000e2757d7fd66f5feeeb3c7643e
Author: Silviu Minut <sminut@cisco.com>
Date:   Fri Oct 1 15:24:07 2021 -0400

    protocols: prevent infinite loop over tcp options

diff --git a/src/protocols/tcp_options.cc b/src/protocols/tcp_options.cc
index 75dc1abfc509e847e87c36e5b81cad24a7df29cc..1cf66aa204248509b16fa00ecd2efaa9d5abdb3e 100644 (file)
--- a/src/protocols/tcp_options.cc
+++ b/src/protocols/tcp_options.cc
@@ -23,17 +23,33 @@
 
 #include "tcp_options.h"
 
+#include "stream/tcp/tcp_module.h"
+
 #include "packet.h"
 #include "tcp.h"
 
+extern THREAD_LOCAL TcpStats tcpStats;
+
 namespace snort
 {
 namespace tcp
 {
-TcpOptIteratorIter::TcpOptIteratorIter(const TcpOption* first_opt) : opt(first_opt) { }
+TcpOptIteratorIter::TcpOptIteratorIter(const TcpOption* first_opt, const TcpOptIterator* it) : opt(first_opt), iter(it) { }
 
 const TcpOption& TcpOptIteratorIter::operator*() const { return *opt; }
 
+const TcpOptIteratorIter& TcpOptIteratorIter::operator++()
+{
+    const auto* old_opt = opt;
+    opt = &opt->next();
+    if (opt == old_opt)       // defend against option length = 0
+    {
+        *this = iter->end();
+        tcpStats.zero_len_tcp_opt++;
+    }
+    return *this;
+}
+
 TcpOptIterator::TcpOptIterator(const TCPHdr* const tcp_header, const Packet* const p)
 {
     const uint8_t* const hdr = (const uint8_t*)tcp_header;
@@ -68,12 +84,12 @@ TcpOptIterator::TcpOptIterator(const TCPHdr* const tcp_header, const uint32_t va
 
 TcpOptIteratorIter TcpOptIterator::begin() const
 {
-    return TcpOptIteratorIter(reinterpret_cast<const TcpOption*>(start_ptr));
+    return TcpOptIteratorIter(reinterpret_cast<const TcpOption*>(start_ptr), this);
 }
 
 TcpOptIteratorIter TcpOptIterator::end() const
 {
-    return TcpOptIteratorIter(reinterpret_cast<const TcpOption*>(end_ptr));
+    return TcpOptIteratorIter(reinterpret_cast<const TcpOption*>(end_ptr), this);
 }
 } // namespace ip
 } // namespace snort

diff --git a/src/protocols/tcp_options.h b/src/protocols/tcp_options.h
index 80feebbb76b16893d14e38c9f7c168ed84729961..fad01028140c4923a838209921642695f7c79100 100644 (file)
--- a/src/protocols/tcp_options.h
+++ b/src/protocols/tcp_options.h
@@ -124,6 +124,8 @@ struct TcpOption
     }
 };
 
+class TcpOptIterator;
+
 /*
  * Use TcpOptIterator ... this should NOT be called directly
  * unless you want to an actual iterator or some buggy code.
@@ -131,7 +133,7 @@ struct TcpOption
 class SO_PUBLIC TcpOptIteratorIter
 {
 public:
-    TcpOptIteratorIter(const TcpOption*);
+    TcpOptIteratorIter(const TcpOption*, const TcpOptIterator*);
 
     bool operator==(const TcpOptIteratorIter& rhs)
     { return opt == rhs.opt; }
@@ -139,16 +141,12 @@ public:
     bool operator!=(const TcpOptIteratorIter& rhs)
     { return opt != rhs.opt; }
 
-    TcpOptIteratorIter& operator++()
-    {
-        opt = &opt->next();
-        return *this;
-    }
-
+    const TcpOptIteratorIter& operator++();
     const TcpOption& operator*() const;
 
 private:
     const TcpOption* opt;
+    const TcpOptIterator* iter;
 };
 
 /*

diff --git a/src/stream/tcp/tcp_module.cc b/src/stream/tcp/tcp_module.cc
index d8dfdef48dc25460180b73885d7045798a9ca760..e0037fdf570479439c0399f394caa7baa31f0b87 100644 (file)
--- a/src/stream/tcp/tcp_module.cc
+++ b/src/stream/tcp/tcp_module.cc
@@ -114,6 +114,7 @@ const PegInfo tcp_pegs[] =
     { CountType::SUM, "partial_fallbacks", "count of fallbacks from assigned service stream splitter" },
     { CountType::MAX, "max_segs", "maximum number of segments queued in any flow" },
     { CountType::MAX, "max_bytes", "maximum number of bytes queued in any flow" },
+    { CountType::SUM, "zero_len_tcp_opt", "number of zero length tcp options" },
     { CountType::END, nullptr, nullptr }
 };
 

diff --git a/src/stream/tcp/tcp_module.h b/src/stream/tcp/tcp_module.h
index 9d9cad9f8c8ddd153d2a531fb4b3d5eccc029156..cb0e95eaec83524af7cf3b7155ac2058d080d5f0 100644 (file)
--- a/src/stream/tcp/tcp_module.h
+++ b/src/stream/tcp/tcp_module.h
@@ -112,6 +112,7 @@ struct TcpStats
     PegCount partial_fallbacks;
     PegCount max_segs;
     PegCount max_bytes;
+    PegCount zero_len_tcp_opt;
 };
 
 extern THREAD_LOCAL struct TcpStats tcpStats;