From d0cc30eccd6de76f39f8520e954e6172f371a168 Mon Sep 17 00:00:00 2001 From: Tim Wojtulewicz Date: Fri, 25 Sep 2020 16:44:27 -0700 Subject: [PATCH] Set data to ip header's payload instead of advancing the pointer --- src/packet_analysis/protocol/ip/IP.cc | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/packet_analysis/protocol/ip/IP.cc b/src/packet_analysis/protocol/ip/IP.cc index bba6750307..aaafa4fe28 100644 --- a/src/packet_analysis/protocol/ip/IP.cc +++ b/src/packet_analysis/protocol/ip/IP.cc @@ -175,13 +175,6 @@ bool IPAnalyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* packet) ip_hdr_len = ip_hdr->HdrLen(); packet->cap_len = total_len + packet->hdr_size; - // TODO: in the old code, the data pointer is updated to point at the IP header's - // payload, so it contains all of the data when it's processed. This isn't a big - // deal for when we pass it down into the session analyzers, since that does the - // same itself. should it be updated here for the case where a fragmented packet - // is actually tunneled? is that a thing that can happen? Does updating the data - // pointer without also updating the one in packet cause any problems? - if ( ip_hdr_len > total_len ) { sessions->Weird("invalid_IP_header_size", ip_hdr.get(), encapsulation); @@ -227,8 +220,9 @@ bool IPAnalyzer::AnalyzePacket(size_t len, const uint8_t* data, Packet* packet) } #endif - // Advance the data pointer past the IP header based on the header length - data += ip_hdr_len; + // Set the data pointer to match the payload from the IP header. This makes sure that it's also pointing + // at the reassembled data for a fragmented packet. + data = ip_hdr->Payload(); len -= ip_hdr_len; bool return_val = true;