From e9c64a89a5915846f0ae1f5720f30a82f76a2e4d Mon Sep 17 00:00:00 2001 From: Pavel Odintsov Date: Fri, 13 Dec 2024 22:02:46 +0300 Subject: [PATCH] Added sanity check in IPFIX code to avoid reading outside of our memory region. Reported by Evgeny Shtanov Closes: #1030 --- src/netflow_plugin/ipfix_collector.cpp | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/netflow_plugin/ipfix_collector.cpp b/src/netflow_plugin/ipfix_collector.cpp index 328ce925..36da4fb2 100644 --- a/src/netflow_plugin/ipfix_collector.cpp +++ b/src/netflow_plugin/ipfix_collector.cpp @@ -1578,19 +1578,22 @@ bool process_ipfix_packet(const uint8_t* packet, return false; } + // Check that we have enough space in packet to read flowset header + if (offset + sizeof(ipfix_flowset_header_common_t) > ipfix_packet_length) { + logger << log4cpp::Priority::ERROR + << "Flowset is too short: we do not have space for flowset header. " + << "IPFIX packet agent IP:" << client_addres_in_string_format + << " flowset number: " << flowset_number << " offset: " << offset << " packet_length: " << ipfix_packet_length; + return false; + } + const ipfix_flowset_header_common_t* flowset = (const ipfix_flowset_header_common_t*)(packet + offset); uint32_t flowset_id = ntohs(flowset->flowset_id); uint32_t flowset_length = ntohs(flowset->length); - /* - * Yes, this is a near duplicate of the short packet check - * above, but this one validates the flowset length from in - * the packet before we pass it to the flowset-specific - * handlers below. - */ - - if (offset + flowset_length > ipfix_packet_length) { + // One more check to ensure that we have enough space in packet to read whole flowset + if (offset + flowset_length > ipfix_packet_length) { logger << log4cpp::Priority::ERROR << "We tried to read from address outside IPFIX packet flowset agent IP: " << client_addres_in_string_format; return false;