From 581971e1604879b992746ea15a48049d15d542e9 Mon Sep 17 00:00:00 2001 From: Christian Kreibich Date: Thu, 25 Apr 2024 16:16:15 -0700 Subject: [PATCH] Factor in caplens in ICMPAnalyzer::DeliverPacket length calculations Relying only on the IP-header-provided length could violate buffer boundaries in the endpoints' rule matching. This change mirrors what we do in UDP and TCP. Resolves #3671 --- src/packet_analysis/protocol/icmp/ICMP.cc | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/packet_analysis/protocol/icmp/ICMP.cc b/src/packet_analysis/protocol/icmp/ICMP.cc index 36c100bf40..d59f409ec4 100644 --- a/src/packet_analysis/protocol/icmp/ICMP.cc +++ b/src/packet_analysis/protocol/icmp/ICMP.cc @@ -92,11 +92,14 @@ void ICMPAnalyzer::DeliverPacket(Connection* c, double t, bool is_orig, int rema c->SetLastTime(run_state::current_timestamp); adapter->InitEndpointMatcher(ip.get(), len, is_orig); - // Move past common portion of ICMP header. + // Move past common portion of ICMP header. BuildConnTuple() verified that + // the header is fully present. data += 8; remaining -= 8; len -= 8; + // The ICMP session adapter only uses len to signal endpoint activity, so + // caplen vs len does not matter. adapter->UpdateLength(is_orig, len); if ( ip->NextProto() == IPPROTO_ICMP ) @@ -112,12 +115,12 @@ void ICMPAnalyzer::DeliverPacket(Connection* c, double t, bool is_orig, int rema // handling those properly. pkt->session = c; - ForwardPacket(len, data, pkt); + ForwardPacket(std::min(len, remaining), data, pkt); if ( remaining >= len ) adapter->ForwardPacket(len, data, is_orig, -1, ip.get(), remaining); - adapter->MatchEndpoint(data, len, is_orig); + adapter->MatchEndpoint(data, std::min(len, remaining), is_orig); } void ICMPAnalyzer::NextICMP4(double t, const struct icmp* icmpp, int len, int caplen, const u_char*& data,