From: Steve Chew Date: Tue, 19 Apr 2016 20:53:45 +0000 (-0400) Subject: Fix some bugs found during testing and uncrustify. X-Git-Tag: 3.0.0-233~420^2~2 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=371a5bac7bfb9b499c84aec2d44dc9faa8b1f119;p=thirdparty%2Fsnort3.git Fix some bugs found during testing and uncrustify. --- diff --git a/src/codecs/ip/cd_frag.cc b/src/codecs/ip/cd_frag.cc index 3f299f386..3b49be239 100644 --- a/src/codecs/ip/cd_frag.cc +++ b/src/codecs/ip/cd_frag.cc @@ -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) diff --git a/src/codecs/ip/cd_no_next.cc b/src/codecs/ip/cd_no_next.cc index c90ad1a0f..bfaf6592a 100644 --- a/src/codecs/ip/cd_no_next.cc +++ b/src/codecs/ip/cd_no_next.cc @@ -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(raw.len) = ip::MIN_EXT_LEN; codec.lyr_len = ip::MIN_EXT_LEN; diff --git a/src/protocols/ipv6.h b/src/protocols/ipv6.h index 5fbbc1685..517c9c9c3 100644 --- a/src/protocols/ipv6.h +++ b/src/protocols/ipv6.h @@ -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 diff --git a/src/protocols/layer.cc b/src/protocols/layer.cc index 3f4b514df..cfd88b924 100644 --- a/src/protocols/layer.cc +++ b/src/protocols/layer.cc @@ -94,7 +94,8 @@ const arp::EtherARP* get_arp_layer(const Packet* const p) const Layer* lyr = p->layers; return reinterpret_cast( - 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(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(lyr.start); api.set(ip6h); curr_layer++; + if(curr_layer >= num_layers) + return false; return true; } default: diff --git a/src/protocols/packet_manager.cc b/src/protocols/packet_manager.cc index 0236ebf1f..070ab7cc1 100644 --- a/src/protocols/packet_manager.cc +++ b/src/protocols/packet_manager.cc @@ -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(lyr->start), c->ptrs); diff --git a/src/protocols/protocol_ids.h b/src/protocols/protocol_ids.h index 430bacbdb..7fd08f252 100644 --- a/src/protocols/protocol_ids.h +++ b/src/protocols/protocol_ids.h @@ -39,7 +39,7 @@ // Convert enum to a value cast to the enum's underlying type. template -constexpr auto to_utype(En t) -> typename std::underlying_type::type +constexpr auto to_utype(En t)->typename std::underlying_type::type { return static_cast::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::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::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)