decode: cleanup packet properly on bad packets

In case of bad IPv4, TCP or UDP, the per packet ip4vars/tcpvars/udpvar
structures would not be cleaned up because the cleanup depends on the
'header' pointer being set, but the error handling would unset that.

This could mean these structures were already filled with values before
the error was detected. As packets were recycled, the next packet decoding
would use this unclean structure.

To make things worse these structures are part of unions. IPv4/IPv6 and
TCP/ICMPv4/ICMPv6 share the same memory location.

LibFuzzer+UBSAN found this both locally and in Oss-Fuzz:

decode-ipv6.c:654:9: runtime error: load of value 6, which is not a valid value for type 'bool'
    #0 0x6146f0 in DecodeIPV6 /src/suricata/src/decode-ipv6.c:654:9
    #1 0x617e96 in DecodeNull /src/suricata/src/decode-null.c:70:13
    #2 0x9dd8a4 in DecodePcapFile /src/suricata/src/source-pcap-file.c:412:9
    #3 0x4c8ed2 in LLVMFuzzerTestOneInput /src/suricata/src/tests/fuzz/fuzz_sigpcap.c:158:25
    #4 0x457e51 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:556:15
    #5 0x457575 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool*) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:470:3
    #6 0x459917 in fuzzer::Fuzzer::MutateAndTestOne() /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:698:19
    #7 0x45a6a5 in fuzzer::Fuzzer::Loop(std::__Fuzzer::vector<fuzzer::SizedFile, fuzzer::fuzzer_allocator<fuzzer::SizedFile> >&) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerLoop.cpp:830:5
    #8 0x448728 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerDriver.cpp:824:6
    #9 0x472552 in main /src/llvm-project/compiler-rt/lib/fuzzer/FuzzerMain.cpp:19:10
    #10 0x7ff0d097b82f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #11 0x41bde8 in _start (/out/fuzz_sigpcap+0x41bde8)

Bug: #3610

diff --git a/src/decode-ipv4.c b/src/decode-ipv4.c
index 47c0e31e7c3a84af8ffb736e8cf0fbe4b6fd3a8f..478c6a66b4aeb0fb530fc1b39983410798b2f229 100644 (file)
--- a/src/decode-ipv4.c
+++ b/src/decode-ipv4.c
@@ -524,7 +524,7 @@ int DecodeIPV4(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p,
     /* do the actual decoding */
     if (unlikely(DecodeIPV4Packet (p, pkt, len) < 0)) {
         SCLogDebug("decoding IPv4 packet failed");
-        p->ip4h = NULL;
+        CLEAR_IPV4_PACKET((p));
         return TM_ECODE_FAILED;
     }
     p->proto = IPV4_GET_IPPROTO(p);

diff --git a/src/decode-ipv6.c b/src/decode-ipv6.c
index 430712ca9a2a8b76a22dddc29c16ffca51ca7525..6b6701fd445148826d2327acb983e4eaa11ca1c5 100644 (file)
--- a/src/decode-ipv6.c
+++ b/src/decode-ipv6.c
@@ -589,7 +589,7 @@ int DecodeIPV6(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p, const uint8_t *
     /* do the actual decoding */
     int ret = DecodeIPV6Packet (tv, dtv, p, pkt, len);
     if (unlikely(ret < 0)) {
-        p->ip6h = NULL;
+        CLEAR_IPV6_PACKET(p);
         return TM_ECODE_FAILED;
     }
 

diff --git a/src/decode-sctp.c b/src/decode-sctp.c
index ba2697b4173800e72e492b7ddc19ea1bf32ba5e6..cb5811ac8c754c5f29783f78625ac5f348ee9850 100644 (file)
--- a/src/decode-sctp.c
+++ b/src/decode-sctp.c
@@ -65,7 +65,7 @@ int DecodeSCTP(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p,
     StatsIncr(tv, dtv->counter_sctp);
 
     if (unlikely(DecodeSCTPPacket(tv, p,pkt,len) < 0)) {
-        p->sctph = NULL;
+        CLEAR_SCTP_PACKET(p);
         return TM_ECODE_FAILED;
     }
 

diff --git a/src/decode-tcp.c b/src/decode-tcp.c
index e3824d33515892bbc27fda4ef1904410a09ee14e..8abf9a64f27ca29f6a084b66edaaa9911ac8a779 100644 (file)
--- a/src/decode-tcp.c
+++ b/src/decode-tcp.c
@@ -238,7 +238,7 @@ int DecodeTCP(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p,
 
     if (unlikely(DecodeTCPPacket(tv, p, pkt,len) < 0)) {
         SCLogDebug("invalid TCP packet");
-        p->tcph = NULL;
+        CLEAR_TCP_PACKET(p);
         return TM_ECODE_FAILED;
     }
 

diff --git a/src/decode-udp.c b/src/decode-udp.c
index c3a8ebbfe15f498de94dd1c2d2b3720d76120f40..7dfbbfb1afc83bb990dde87ba1adbdd7b31eb11e 100644 (file)
--- a/src/decode-udp.c
+++ b/src/decode-udp.c
@@ -77,7 +77,7 @@ int DecodeUDP(ThreadVars *tv, DecodeThreadVars *dtv, Packet *p,
     StatsIncr(tv, dtv->counter_udp);
 
     if (unlikely(DecodeUDPPacket(tv, p, pkt,len) < 0)) {
-        p->udph = NULL;
+        CLEAR_UDP_PACKET(p);
         return TM_ECODE_FAILED;
     }