]> git.ipfire.org Git - thirdparty/snort3.git/commitdiff
Fix some bugs found during testing and uncrustify.
authorSteve Chew <stechew@cisco.com>
Tue, 19 Apr 2016 20:53:45 +0000 (16:53 -0400)
committerSteve Chew <stechew@cisco.com>
Tue, 19 Apr 2016 21:49:11 +0000 (17:49 -0400)
src/codecs/ip/cd_frag.cc
src/codecs/ip/cd_no_next.cc
src/protocols/ipv6.h
src/protocols/layer.cc
src/protocols/packet_manager.cc
src/protocols/protocol_ids.h

index 3f299f38627e6d560f8813d0dd8df544d90832f0..3b49be239307615f8941a7d892e1226672e7266e 100644 (file)
@@ -98,9 +98,17 @@ bool Ipv6FragCodec::decode(const RawData& raw, CodecData& codec, DecodeData& sno
     codec.proto_bits |= PROTO_BIT__IP6_EXT;
     codec.ip6_extension_count++;
 
+    // FIXIT-H: The comment says to call it after setting next_prot_id,
+    //          but it looks like it's called (twice) before setting it.
+
     // must be called AFTER setting next_prot_id
     CheckIPv6ExtensionOrder(codec, IpProtocol::FRAGMENT);
 
+    //   FIXIT-H:
+    //   This breaks the tests/ips/normalize/ip6/would_opts_nop test because
+    //   ip6frag_hdr->ip6f_nxt is set to FINISHED_DECODE here.
+    //   (or maybe the test has the wrong expected data).
+
     // Since the Frag layer is removed from rebuilt packets, ensure
     // the next layer is correctly order now.
     if (frag_offset == 0)
index c90ad1a0f5ccc563f4458cc54380ca2957618773..bfaf6592a45d7a56704ce71b7b3271721cc590df 100644 (file)
@@ -44,7 +44,7 @@ public:
 
 bool Ipv6NoNextCodec::decode(const RawData& raw, CodecData& codec, DecodeData&)
 {
-    // No need ot check IPv6 extension order since this is automatically
+    // No need to check IPv6 extension order since this is automatically
     // the last extension.
     if (raw.len < ip::MIN_EXT_LEN)
         return false;
@@ -55,7 +55,10 @@ bool Ipv6NoNextCodec::decode(const RawData& raw, CodecData& codec, DecodeData&)
         return false;
     }
 
-    // The size of this packets data should be zero.  So, set this layer's
+    // FIXIT-M: What if the packet's data is non-zero?  See allpcaps3.pcap
+    //          Should this be logged as an event?
+
+    // The size of this packet's data should be zero.  So, set this layer's
     // length and the packet's remaining length to the same number.
     const_cast<uint32_t&>(raw.len) = ip::MIN_EXT_LEN;
     codec.lyr_len = ip::MIN_EXT_LEN;
index 5fbbc168537babe6edd076f05fccad2b659173f7..517c9c9c3163db13501ec7009f6eeb8882d84732 100644 (file)
@@ -136,17 +136,17 @@ struct IP6Hdr
     {
         switch (ip6_next)
         {
-            case IpProtocol::NONEXT:
-            case IpProtocol::TCP:
-            case IpProtocol::UDP:
-            case IpProtocol::ICMPV6:
-            case IpProtocol::HOPOPTS:
-            case IpProtocol::DSTOPTS:
-            case IpProtocol::ROUTING:
-            case IpProtocol::FRAGMENT:
-                return true;
-            default:
-                break;
+        case IpProtocol::NONEXT:
+        case IpProtocol::TCP:
+        case IpProtocol::UDP:
+        case IpProtocol::ICMPV6:
+        case IpProtocol::HOPOPTS:
+        case IpProtocol::DSTOPTS:
+        case IpProtocol::ROUTING:
+        case IpProtocol::FRAGMENT:
+            return true;
+        default:
+            break;
         }
         return false;
     }
@@ -165,7 +165,6 @@ struct IP6Hdr
 
     inline uint16_t raw_len() const
     { return ip6_payload_len; }
-
 };
 
 enum class HopByHopOptions : uint8_t
@@ -248,11 +247,11 @@ inline int IPV6IdExtensionOrder(const ProtocolId prot_id)
     default:                   return IPV6_ORDER_MAX;
     }
 }
+
 inline int IPV6ExtensionOrder(const IpProtocol ip_proto)
 {
     return IPV6IdExtensionOrder((ProtocolId)ip_proto);
 }
-
 } // namespace ipv6
 
 #endif
index 3f4b514dfb9bdbe8ad54b1dfca07425d80fddd96..cfd88b92457e971fec881cc39c1ccef1daa32c20 100644 (file)
@@ -94,7 +94,8 @@ const arp::EtherARP* get_arp_layer(const Packet* const p)
     const Layer* lyr = p->layers;
 
     return reinterpret_cast<const arp::EtherARP*>(
-        find_inner_layer(lyr, num_layers, ProtocolId::ETHERTYPE_ARP, ProtocolId::ETHERTYPE_REVARP));
+        find_inner_layer(lyr, num_layers, ProtocolId::ETHERTYPE_ARP, 
+            ProtocolId::ETHERTYPE_REVARP));
 }
 
 const gre::GREHdr* get_gre_layer(const Packet* const p)
@@ -347,6 +348,8 @@ bool set_outer_ip_api(const Packet* const p,
                 reinterpret_cast<const ip::IP4Hdr*>(lyr.start);
             api.set(ip4h);
             curr_layer++;
+            if(curr_layer >= num_layers)
+                return false;
             return true;
         }
         case ProtocolId::ETHERTYPE_IPV6:
@@ -356,6 +359,8 @@ bool set_outer_ip_api(const Packet* const p,
                 reinterpret_cast<const ip::IP6Hdr*>(lyr.start);
             api.set(ip6h);
             curr_layer++;
+            if(curr_layer >= num_layers)
+                return false;
             return true;
         }
         default:
index 0236ebf1fde603a53829bb8e693a7c9676151e5e..070ab7cc14e628e988bd2e209850a7324fff2b6f 100644 (file)
@@ -249,7 +249,8 @@ void PacketManager::decode(
             }
             else
             {
-                p->ip_proto_next = convert_protocolid_to_ipprotocol(codec_data.next_prot_id);
+                if(codec_data.next_prot_id != ProtocolId::FINISHED_DECODE)
+                    p->ip_proto_next = convert_protocolid_to_ipprotocol(codec_data.next_prot_id);
             }
         }
 
@@ -311,7 +312,8 @@ void PacketManager::decode(
         }
         else
         {
-            if ( (p->num_layers > 0) && (p->layers[p->num_layers-1].prot_id == ProtocolId::TEREDO) &&
+            if ( (p->num_layers > 0) && 
+                (p->layers[p->num_layers-1].prot_id == ProtocolId::TEREDO) &&
                 (prev_prot_id == ProtocolId::IPV6) )
             {
                 pop_teredo(p, raw);
@@ -433,7 +435,8 @@ bool PacketManager::encode(const Packet* p,
         for (int i = outer_layer; i > inner_layer; --i)
         {
             const Layer& l = lyrs[i];
-            ProtocolIndex mapped_prot = i ? CodecManager::s_proto_map[to_utype(l.prot_id)] : CodecManager::grinder;
+            ProtocolIndex mapped_prot = 
+                i ? CodecManager::s_proto_map[to_utype(l.prot_id)] : CodecManager::grinder;
             if (!CodecManager::s_protocols[mapped_prot]->encode(l.start, l.length, enc, buf))
             {
                 return false;
@@ -449,7 +452,8 @@ bool PacketManager::encode(const Packet* p,
     for (int i = outer_layer; i >= 0; --i)
     {
         const Layer& l = lyrs[i];
-        ProtocolIndex mapped_prot = i ? CodecManager::s_proto_map[to_utype(l.prot_id)] : CodecManager::grinder;
+        ProtocolIndex mapped_prot = 
+            i ? CodecManager::s_proto_map[to_utype(l.prot_id)] : CodecManager::grinder;
 
         if (!CodecManager::s_protocols[mapped_prot]->encode(l.start, l.length, enc, buf))
         {
@@ -763,7 +767,8 @@ int PacketManager::encode_format(
 
         // NOTE: this must always go from outer to inner
         //       to ensure a valid ip header
-        ProtocolIndex mapped_prot = i ? CodecManager::s_proto_map[to_utype(lyr->prot_id)] : CodecManager::grinder;
+        ProtocolIndex mapped_prot = 
+            i ? CodecManager::s_proto_map[to_utype(lyr->prot_id)] : CodecManager::grinder;
 
         CodecManager::s_protocols[mapped_prot]->format(
             reverse, const_cast<uint8_t*>(lyr->start), c->ptrs);
index 430bacbdb902ac1031f0a40b109b30f2e332dd24..7fd08f252eb4e863b2ddb07f06c8f1b0ee08b9a2 100644 (file)
@@ -39,7 +39,7 @@
 
 //  Convert enum to a value cast to the enum's underlying type.
 template<typename En>
-constexpr auto to_utype(En t) -> typename std::underlying_type<En>::type
+constexpr auto to_utype(En t)->typename std::underlying_type<En>::type
 {
     return static_cast<typename std::underlying_type<En>::type>(t);
 }
@@ -79,11 +79,10 @@ enum class IpProtocol : std::uint8_t
     MIN_UNASSIGNED_IP_PROTO = 143,
 
     RESERVED = 255,       // == 0xFF
-    PORT_SCAN = 255, 
+    PORT_SCAN = 255,
     PROTO_NOT_SET = 255,  // Indicates protocol has not been set.
 };
 
-
 //  Values up to 255 MUST be identical to those in IpProtocol.
 enum class ProtocolId : std::uint16_t
 {
@@ -114,7 +113,7 @@ enum class ProtocolId : std::uint16_t
     MIN_UNASSIGNED_IP_PROTO = 143,
 
     RESERVED = 255,       // == 0xFF
-    PORT_SCAN = 255, 
+    PORT_SCAN = 255,
     PROTO_NOT_SET = 255,  // Indicates protocol has not been set.
 
     /*
@@ -153,58 +152,8 @@ enum class ProtocolId : std::uint16_t
     ETHERTYPE_FPATH = 0x8903,
 };
 
-static const auto max_protocol_id = std::numeric_limits<std::underlying_type<ProtocolId>::type>::max();
-
-#if 0
-constexpr uint16_t IPPROTO_ID_HOPOPTS = 0;
-constexpr uint16_t IPPROTO_ID_ICMPV4 = 1;
-constexpr uint16_t IPPROTO_ID_IPIP = 4;
-constexpr uint16_t IPPROTO_ID_TCP = 6;
-constexpr uint16_t IPPROTO_ID_UDP = 17;
-constexpr uint16_t IPPROTO_ID_IPV6 = 41;
-constexpr uint16_t IPPROTO_ID_ROUTING = 43;
-constexpr uint16_t IPPROTO_ID_FRAGMENT = 44;
-constexpr uint16_t IPPROTO_ID_GRE = 47;
-constexpr uint16_t IPPROTO_ID_ESP = 50;
-constexpr uint16_t IPPROTO_ID_AUTH = 51; // RFC 4302
-constexpr uint16_t IPPROTO_ID_MOBILITY = 55;
-constexpr uint16_t IPPROTO_ID_ICMPV6 = 58;
-constexpr uint16_t IPPROTO_ID_NONEXT = 59;
-constexpr uint16_t IPPROTO_ID_DSTOPTS = 60;
-constexpr uint16_t IPPROTO_ID_RESERVED = 255; // == 0xFF
-
-constexpr uint16_t MIN_UNASSIGNED_IP_PROTO = 143;
-
-/*
- *  Undefined Protocol!
- */
-constexpr uint16_t FINISHED_DECODE = 0x0100;  // Indicates Codecs have succesfully decoded packet
-constexpr uint16_t PROTO_TEREDO = 0x0101;
-constexpr uint16_t PROTO_GTP = 0x0102;
-constexpr uint16_t PROTO_IP_EMBEDDED_IN_ICMP4 = 0x0103;
-constexpr uint16_t PROTO_IP_EMBEDDED_IN_ICMP6 = 0x0104;
-constexpr uint16_t PROTO_ETHERNET_802_3 = 0x0105;
-constexpr uint16_t PROTO_ETHERNET_802_11 = 0x0106;
-constexpr uint16_t PROTO_ETHERNET_LLC = 0x0107;
-
-/*
- * Below is a partial list of ethertypes.
- *  Defined at:
- *
- *  Defined at:
- * http://www.iana.org/assignments/ieee-802-numbers/ieee-802-numbers.xhtml
- */
-constexpr uint16_t ETHERTYPE_TRANS_ETHER_BRIDGING = 0x6558;
-constexpr uint16_t ETHERTYPE_IPV4 = 0x0800;
-constexpr uint16_t ETHERTYPE_REVARP = 0x8035;
-constexpr uint16_t ETHERTYPE_ARP = 0x0806;
-constexpr uint16_t ETHERTYPE_8021Q = 0x8100;
-constexpr uint16_t ETHERTYPE_IPX = 0x8137;
-constexpr uint16_t ETHERTYPE_IPV6 = 0x86dd;
-constexpr uint16_t ETHERTYPE_PPP = 0x880B;
-constexpr uint16_t ETHERTYPE_EAPOL = 0x888e;
-constexpr uint16_t ETHERTYPE_FPATH = 0x8903;
-#endif
+static const auto max_protocol_id =
+    std::numeric_limits<std::underlying_type<ProtocolId>::type>::max();
 
 inline bool is_ip_protocol(ProtocolId prot_id)
 {
@@ -220,7 +169,7 @@ inline IpProtocol convert_protocolid_to_ipprotocol(const ProtocolId prot_id)
 
 inline bool is_ip6_extension(const ProtocolId prot_id)
 {
-    if(!is_ip_protocol(prot_id))
+    if (!is_ip_protocol(prot_id))
         return false;
 
     switch (prot_id)