Merge pull request #2294 in SNORT/snort3 from ~MASHASAN/snort3:ft_icmp_test to master

Squashed commit of the following:

commit 6c3206696220bec935f7e1beb6699dcc5b14bd3d
Author: Masud Hasan <mashasan@cisco.com>
Date:   Thu Jun 25 22:53:59 2020 -0400

    stream_ip: Avoid modifying the original fragmented packet during rebuild

diff --git a/src/protocols/packet_manager.cc b/src/protocols/packet_manager.cc
index 16d2a96b003df713edd3b2f7f8bdd4046e6960fd..065de7a3e13d40952188801e77c83e12e4bc45ee 100644 (file)
--- a/src/protocols/packet_manager.cc
+++ b/src/protocols/packet_manager.cc
@@ -685,28 +685,13 @@ int PacketManager::encode_format(
     EncodeFlags f, const Packet* p, Packet* c, PseudoPacketType type,
     const DAQ_PktHdr_t* phdr, uint32_t opaque)
 {
-    int i;
-    int len;
     bool update_ip4_len = false;
-    uint8_t num_layers = p->num_layers;
-
-    if ( num_layers == 0 )
-        return -1;
-
-    c->reset();
-    init_daq_pkthdr(p, c, phdr, opaque);
-
-    if ( f & ENC_FLAG_NET )
-    {
-        num_layers = layer::get_inner_ip_lyr_index(p) + 1;
+    uint8_t num_layers;
 
-        if (num_layers == 0) // FIXIT-L is this an extraneous check?
-            return -1;
-    }
-    else if ( f & ENC_FLAG_DEF )
+    if ( f & ENC_FLAG_DEF )
     {
         /*
-         * By its definitinos, this flag means 'stop before innermost ip4
+         * By its definitions, this flag means 'stop before innermost ip4
          * opts or ip6 frag header'. So, stop after the ip4 layer IP4 will format itself, and now
          * we ensure that the ip6_frag header is not copied too.
          */
@@ -720,28 +705,26 @@ int PacketManager::encode_format(
             num_layers = layer::get_inner_ip_lyr_index(p) + 1;
             update_ip4_len = true;
         }
-
-        if (num_layers == 0)
-            return -1;
     }
+    else if ( f & ENC_FLAG_NET )
+        num_layers = layer::get_inner_ip_lyr_index(p) + 1;
+    else
+        num_layers = p->num_layers;
+
+    if ( num_layers == 0 )
+        return -1;
+
+    init_daq_pkthdr(p, c, phdr, opaque);
 
     // copy raw packet data to clone
     Layer* lyr = (Layer*)p->layers + num_layers - 1;
-    len = lyr->start - p->pkt + lyr->length;
+    int len = lyr->start - p->pkt + lyr->length;
     memcpy((void*)c->pkt, p->pkt, len);
 
-    if ( update_ip4_len )
-    {
-        ip::IP4Hdr* ip4h = reinterpret_cast<ip::IP4Hdr*>(const_cast<uint8_t*>(lyr->start));
-        lyr->length = ip::IP4_HEADER_LEN;
-        ip4h->set_ip_len(ip::IP4_HEADER_LEN);
-        ip4h->set_hlen(ip::IP4_HEADER_LEN >> 2);
-    }
-
     const bool reverse = !(f & ENC_FLAG_FWD);
 
     // set up and format layers
-    for ( i = 0; i < num_layers; i++ )
+    for ( int i = 0; i < num_layers; i++ )
     {
         const uint8_t* b = c->pkt + (p->layers[i].start - p->pkt); // == c->pkt + p->layers[i].len
         lyr = c->layers + i;
@@ -759,6 +742,15 @@ int PacketManager::encode_format(
             reverse, const_cast<uint8_t*>(lyr->start), c->ptrs);
     }
 
+    if ( update_ip4_len )
+    {
+        lyr = (Layer*)c->layers + num_layers - 1;
+        ip::IP4Hdr* ip4h = reinterpret_cast<ip::IP4Hdr*>(const_cast<uint8_t*>(lyr->start));
+        lyr->length = ip::IP4_HEADER_LEN;
+        ip4h->set_ip_len(ip::IP4_HEADER_LEN);
+        ip4h->set_hlen(ip::IP4_HEADER_LEN >> 2);
+    }
+
     // setup payload info
     c->num_layers = num_layers;
     c->data = lyr->start + lyr->length;

diff --git a/src/stream/ip/ip_defrag.cc b/src/stream/ip/ip_defrag.cc
index b764135355ad2229ee0b2db2dc5ddfcd4a96eb88..8703e5fb13ccfffba4a4e4d738e91e03ca9de759 100644 (file)
--- a/src/stream/ip/ip_defrag.cc
+++ b/src/stream/ip/ip_defrag.cc
@@ -359,8 +359,8 @@ static inline int FragCheckFirstLast(
                 break;
             }
         }
-
-        ft->frag_flags |= FRAG_GOT_LAST;
+        else
+            ft->frag_flags |= FRAG_GOT_LAST;
 
         /*
          * If this is the last frag (and we don't have a frag that already