Merge pull request #952 in SNORT/snort3 from fixups to master

Squashed commit of the following:

commit e6a65d1395eaa1f5da5c5f7b3f3e8e713de161c3
Author: Russ Combs <rucombs@cisco.com>
Date:   Mon Jul 10 19:11:07 2017 -0400

    ip and tcp options: reformat for consistency

commit 8dd7f558b4d5c8eb890f93e0635c959b43de5a18
Author: Russ Combs <rucombs@cisco.com>
Date:   Mon Jul 10 16:11:59 2017 -0400

    ip and tcp options: print the correct octets

commit e9eb69680b352445e7c0d55211e926ef7f6913b1
Author: Russ Combs <rucombs@cisco.com>
Date:   Mon Jul 10 11:25:22 2017 -0400

    detect: release any helpers from an undetected PDU upon finish

commit 917e02259ad61b1fde19641acbb9d8095cc18741
Author: Russ Combs <rucombs@cisco.com>
Date:   Mon Jul 10 11:24:29 2017 -0400

    ip and tcp options: use max opts len to squelch bogus reinterpret cast overrun

diff --git a/src/detection/detection_engine.cc b/src/detection/detection_engine.cc
index 128672e7b3add109056d3e0a999273df53eb958a..b976cbe9e46dd11d92910096da13325c6ff69409 100644 (file)
--- a/src/detection/detection_engine.cc
+++ b/src/detection/detection_engine.cc
@@ -128,12 +128,11 @@ void DetectionEngine::finish_packet(Packet* p)
 {
     log_events(p);
     clear_events(p);
+    p->release_helpers();
 
-    if ( p->endianness )
-    {
-        delete p->endianness;
-        p->endianness = nullptr;
-    }
+    // clean up any failed rebuilds
+    const IpsContext* c = Snort::get_switcher()->get_next();
+    c->packet->release_helpers();
 }
 
 uint8_t* DetectionEngine::get_buffer(unsigned& max)

diff --git a/src/log/log_text.cc b/src/log/log_text.cc
index 3557638d118b7bd1cc22dfe26f5a7e849e579379..1defaecb8b79d46e82f15c84d6b9978508e834d8 100644 (file)
--- a/src/log/log_text.cc
+++ b/src/log/log_text.cc
@@ -142,8 +142,6 @@ void Log2ndHeader(TextLog* log, Packet* p)
 
 static void LogIpOptions(TextLog* log, const ip::IpOptionIterator& options)
 {
-    int print_offset;
-    int init_offset = TextLog_Tell(log);
     unsigned c = 0;
 
     for (const auto& opt : options)
@@ -160,14 +158,6 @@ static void LogIpOptions(TextLog* log, const ip::IpOptionIterator& options)
 
     for (auto op : options)
     {
-        print_offset = TextLog_Tell(log);
-
-        if ((print_offset - init_offset) > 60)
-        {
-            TextLog_Puts(log, "\nIP Options => ");
-            init_offset = TextLog_Tell(log);
-        }
-
         switch (op.code)
         {
         case ip::IPOptionCodes::RR:
@@ -212,22 +202,19 @@ static void LogIpOptions(TextLog* log, const ip::IpOptionIterator& options)
             break;
 
         default:
-            TextLog_Print(log, "Opt %d: ", (int)op.code);
+            // the only cases where op.len is invalid were handled aboved
+            // op.len includes code and length bytes but data does not
+            TextLog_Print(log, "Type %u, Len %u", op.code, op.len);
 
-            // the only cases where len is invalid were handled aboved
-            const uint8_t opt_len = op.len;
-            int j;
+            if ( op.len <= 2 )
+                break;
 
-            for (j = 0; (j + 1) < opt_len; j += 2)
-            {
-                TextLog_Print(log, "%02X%02X ",op.data[j],
-                    op.data[j+1]);
-            }
+            TextLog_Print(log, " |%02X", op.data[0]);
+
+            for ( unsigned j = 1, n = op.len - 2; j < n; ++j )
+                TextLog_Print(log, " %02X", op.data[j]);
 
-            // since we're skipping by two, if (j+1) == opt_len,
-            // we will not have printed j
-            if (j < opt_len)
-                TextLog_Print(log, "%02X", op.data[j]);
+            TextLog_Print(log, "| ");
             break;
         }
     }
@@ -460,15 +447,6 @@ static void LogTcpOptions(TextLog* log, const tcp::TcpOptIterator& opt_iter)
 
     for (const tcp::TcpOption& opt : opt_iter)
     {
-#if 0
-        print_offset = TextLog_Tell(log);
-
-        if ((print_offset - init_offset) > 60)
-        {
-            TextLog_Puts(log, "\nTCP Options => ");
-            init_offset = TextLog_Tell(log);
-        }
-#endif
         switch (opt.code)
         {
         case tcp::TcpOptCode::MAXSEG:
@@ -541,31 +519,19 @@ static void LogTcpOptions(TextLog* log, const tcp::TcpOptIterator& opt_iter)
             break;
 
         default:
-        {
-            const int opt_len = opt.len - 2;
+            TextLog_Print(log, " Kind %u, Len %u", opt.code, opt.len);
 
-            if (opt_len > 0)
-            {
-                TextLog_Print(log, "  Opt %d (%d):", opt.code,
-                    (int)opt_len);
+            if ( opt.len <= 2 )
+                break;
 
-                for (int i = 0; (i + 1) < opt_len; i += 2)
-                {
-                    TextLog_Print(log, " %02X%02X",  opt.data[i],
-                        opt.data[i+1]);
-                }
+            TextLog_Print(log, " |%02X",  opt.data[0]);
 
-                // if there is an odd number of bytes
-                if (opt_len & 1)
-                    TextLog_Print(log, " %02x", opt.data[opt_len - 1]);
-            }
-            else
-            {
-                TextLog_Print(log, "  Opt %d", opt.code);
-            }
+            for ( unsigned i = 1, n = opt.len - 2; i < n; ++i )
+                TextLog_Print(log, " %02X",  opt.data[i]);
+
+            TextLog_Print(log, "|");
             break;
         }
-        }
     }
 }
 

diff --git a/src/protocols/ipv4_options.h b/src/protocols/ipv4_options.h
index 5f90dedfd5259a1794fe8df90f67914d11eeef7e..6f24e25f6596375a181473fedd6a8e58eb90d25d 100644 (file)
--- a/src/protocols/ipv4_options.h
+++ b/src/protocols/ipv4_options.h
@@ -44,11 +44,13 @@ enum class IPOptionCodes : std::uint8_t
     ANY = 0xff,
 };
 
+// FIXIT-L reduce all these classes to a simple pointer based approach
+// that doesn't require any reinterpret casts (see also tcp_options.h)
 struct IpOptions
 {
     IPOptionCodes code;
     uint8_t len;
-    uint8_t data[6];  // arbitrary number. choosing six to align with 64 bits
+    uint8_t data[40];  // maximum possible
 
     inline uint8_t get_len() const
     { return ((uint8_t)code <= 1) ? 1 : len; }

diff --git a/src/protocols/packet.cc b/src/protocols/packet.cc
index 3139265e7095f869de75cad789ca70d85a84d674..3f4a36e30637abfa09c60c25dc30533e421176f2 100644 (file)
--- a/src/protocols/packet.cc
+++ b/src/protocols/packet.cc
@@ -55,24 +55,17 @@ Packet::Packet(bool packet_data)
 
 Packet::~Packet()
 {
-    if (obfuscator)
-        delete obfuscator;
+    release_helpers();
+
     if (allocated)
         delete[] (uint8_t*)pkth;
+
     delete[] layers;
 }
 
 void Packet::reset()
 {
-    if ( obfuscator )
-        delete obfuscator;
-
-    if ( endianness )
-        delete endianness;  // FIXIT-L dce2 leaks in a few cases
-
     flow = nullptr;
-    endianness = nullptr;
-    obfuscator = nullptr;
     packet_flags = 0;
     xtradata_mask = 0;
     proto_bits = 0;
@@ -81,9 +74,25 @@ void Packet::reset()
     ip_proto_next = IpProtocol::PROTO_NOT_SET;
     disable_inspect = false;
 
+    release_helpers();
     ptrs.reset();
 }
 
+void Packet::release_helpers()
+{
+    if ( obfuscator )
+    {
+        delete obfuscator;
+        obfuscator = nullptr;
+    }
+
+    if ( endianness )
+    {
+        delete endianness;
+        endianness = nullptr;
+    }
+}
+
 bool Packet::get_ip_proto_next(uint8_t& lyr, IpProtocol& proto) const
 {
     if (lyr > num_layers)

diff --git a/src/protocols/packet.h b/src/protocols/packet.h
index 51dbf634debd407fa8258a6f0344dd18259ec346..e9c85331f18777592e5829ba0e6317f755b5fd71 100644 (file)
--- a/src/protocols/packet.h
+++ b/src/protocols/packet.h
@@ -219,6 +219,7 @@ struct SO_PUBLIC Packet
     bool get_ip_proto_next(uint8_t& lyr, IpProtocol& proto) const;
 
     void reset();
+    void release_helpers();
 
     bool is_from_client() const
     { return (packet_flags & PKT_FROM_CLIENT) != 0; }

diff --git a/src/protocols/tcp_options.h b/src/protocols/tcp_options.h
index 9210c084b4a011f6139471a45832bf82e7b3ba3d..bfc57f0fb2018b5e59fc504384b4eb434c7603a9 100644 (file)
--- a/src/protocols/tcp_options.h
+++ b/src/protocols/tcp_options.h
@@ -91,11 +91,13 @@ const uint8_t TCPOLEN_CC_ECHO = 6;  /* page 17 of rfc1644 */
 const uint8_t TCPOLEN_TRAILER_CSUM = 3;
 const uint8_t TCPOLEN_MD5SIG = 18;
 
+// FIXIT-L reduce all these classes to a simple pointer based approach
+// that doesn't require any reinterpret casts (see also ipv4_options.h)
 struct TcpOption
 {
     TcpOptCode code;
     uint8_t len;
-    uint8_t data[13];  // arbitrary number. choosing 13 to align with 128 bits
+    uint8_t data[40];  // maximum possible
 
     inline uint8_t get_len() const
     { return ((uint8_t)code <= 1) ? 1 : len; }