From 415f13a38b24766817aee5ccc94cb696e72ef254 Mon Sep 17 00:00:00 2001 From: seladb Date: Tue, 15 Aug 2023 22:47:33 -0700 Subject: [PATCH 1/3] Auto pre-commit update (#1181) Co-authored-by: GitHub --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 590c1cc1ac..e68a0fddfd 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -25,7 +25,7 @@ repos: - id: codespell pass_filenames: false - repo: https://github.com/crate-ci/typos - rev: v1.16.1 + rev: v1.16.5 hooks: - id: typos args: ['--config=typos-config.toml'] From 459477108bd56eea82216f04b9e157687c631f6c Mon Sep 17 00:00:00 2001 From: "Liu, An-Chi" Date: Wed, 16 Aug 2023 15:07:00 +0900 Subject: [PATCH 2/3] update document to remove doxygen warning Co-authored-by: seladb --- Common++/header/SystemUtils.h | 1 - Packet++/header/DhcpV6Layer.h | 2 -- Packet++/header/DnsLayer.h | 1 - Pcap++/header/Device.h | 1 - Pcap++/header/DpdkDeviceList.h | 1 - Pcap++/header/PcapDevice.h | 1 - Pcap++/header/PcapFileDevice.h | 1 - Pcap++/header/PcapFilter.h | 2 -- Pcap++/header/PcapLiveDevice.h | 2 +- 9 files changed, 1 insertion(+), 11 deletions(-) diff --git a/Common++/header/SystemUtils.h b/Common++/header/SystemUtils.h index a138fe8743..183f82c7ce 100644 --- a/Common++/header/SystemUtils.h +++ b/Common++/header/SystemUtils.h @@ -304,7 +304,6 @@ namespace pcpp * Static init method which should be called once at the beginning of the main method. * @param[in] argc The argc param from main() * @param[in] argv The argv param from main() - * @return No return value */ // cppcheck-suppress constParameter static void init(int argc, char* argv[]) diff --git a/Packet++/header/DhcpV6Layer.h b/Packet++/header/DhcpV6Layer.h index ac5a5afbba..0799cb99a8 100644 --- a/Packet++/header/DhcpV6Layer.h +++ b/Packet++/header/DhcpV6Layer.h @@ -309,7 +309,6 @@ namespace pcpp /** * Set the message type for this layer * @param[in] messageType The message type to set - * @return No return value */ void setMessageType(DhcpV6MessageType messageType); @@ -321,7 +320,6 @@ namespace pcpp /** * Set the transaction ID for this DHCPv6 message * @param[in] transactionId The transaction ID value to set - * @return No return value */ void setTransactionID(uint32_t transactionId) const; diff --git a/Packet++/header/DnsLayer.h b/Packet++/header/DnsLayer.h index 172db06a73..fc81680118 100644 --- a/Packet++/header/DnsLayer.h +++ b/Packet++/header/DnsLayer.h @@ -420,7 +420,6 @@ namespace pcpp /** * Does nothing for this layer - * @return No return value */ virtual void computeCalculateFields() {} diff --git a/Pcap++/header/Device.h b/Pcap++/header/Device.h index fb74935586..44e945d7dc 100644 --- a/Pcap++/header/Device.h +++ b/Pcap++/header/Device.h @@ -41,7 +41,6 @@ namespace pcpp /** * Close the device - * @return No return value */ virtual void close() = 0; diff --git a/Pcap++/header/DpdkDeviceList.h b/Pcap++/header/DpdkDeviceList.h index 479318dc63..e7e441cd0a 100644 --- a/Pcap++/header/DpdkDeviceList.h +++ b/Pcap++/header/DpdkDeviceList.h @@ -48,7 +48,6 @@ namespace pcpp /** * An abstract method that must be implemented by child class. It's the indication for the worker to stop running. After * this method is called the caller expects the worker to stop running as fast as possible - * @return No return value */ virtual void stop() = 0; diff --git a/Pcap++/header/PcapDevice.h b/Pcap++/header/PcapDevice.h index 555c4ff5bd..0471472ebb 100644 --- a/Pcap++/header/PcapDevice.h +++ b/Pcap++/header/PcapDevice.h @@ -54,7 +54,6 @@ namespace pcpp /** * Get statistics from the device * @param[out] stats An object containing the stats - * @return No return value */ virtual void getStatistics(PcapStats& stats) const = 0; diff --git a/Pcap++/header/PcapFileDevice.h b/Pcap++/header/PcapFileDevice.h index f39e81b82f..fdad0a4c45 100644 --- a/Pcap++/header/PcapFileDevice.h +++ b/Pcap++/header/PcapFileDevice.h @@ -42,7 +42,6 @@ namespace pcpp /** * Close the file - * @return No return value */ virtual void close(); }; diff --git a/Pcap++/header/PcapFilter.h b/Pcap++/header/PcapFilter.h index 60495452cd..6e933a5827 100644 --- a/Pcap++/header/PcapFilter.h +++ b/Pcap++/header/PcapFilter.h @@ -142,7 +142,6 @@ namespace pcpp /** * A method that parses the class instance into BPF string format * @param[out] result An empty string that the parsing will be written into. If the string isn't empty, its content will be overridden - * @return No return value */ virtual void parseToString(std::string& result) = 0; @@ -179,7 +178,6 @@ namespace pcpp * A method that parses the class instance into BPF string format * @param[out] result An empty string that the parsing will be written into. If the string isn't empty, its content will be overridden * If the filter is not valid the result will be an empty string - * @return No return value */ virtual void parseToString(std::string& result); diff --git a/Pcap++/header/PcapLiveDevice.h b/Pcap++/header/PcapLiveDevice.h index 0226ade88f..06fb03c6f1 100644 --- a/Pcap++/header/PcapLiveDevice.h +++ b/Pcap++/header/PcapLiveDevice.h @@ -45,7 +45,7 @@ namespace pcpp * @param[in] userCookie A pointer to the object put by the user when packet capturing stared * @return True when main thread should stop blocking or false otherwise */ - typedef bool (*OnPacketArrivesStopBlocking)(RawPacket* pPacket, PcapLiveDevice* pDevice, void* userData); + typedef bool (*OnPacketArrivesStopBlocking)(RawPacket* pPacket, PcapLiveDevice* pDevice, void* userCookie); /** From aa4e62d5d3ae94dfd96c52a98414a24fd187c2a6 Mon Sep 17 00:00:00 2001 From: "Aengus.Jiang" Date: Wed, 16 Aug 2023 14:24:32 +0800 Subject: [PATCH 3/3] Fix dnslayer memory overflow when add Resource (#1165) --- Packet++/header/DnsResourceData.h | 2 +- Packet++/src/DnsLayer.cpp | 2 +- Packet++/src/DnsResource.cpp | 10 +++++----- Packet++/src/DnsResourceData.cpp | 8 ++++---- Tests/Packet++Test/TestDefinition.h | 1 + Tests/Packet++Test/Tests/DnsTests.cpp | 12 ++++++++++++ Tests/Packet++Test/main.cpp | 1 + 7 files changed, 25 insertions(+), 11 deletions(-) diff --git a/Packet++/header/DnsResourceData.h b/Packet++/header/DnsResourceData.h index 9acd8a7631..ee44f92405 100644 --- a/Packet++/header/DnsResourceData.h +++ b/Packet++/header/DnsResourceData.h @@ -363,7 +363,7 @@ namespace pcpp * copied from this byte array to the object * @param[in] dataLen The byte array size */ - GenericDnsResourceData(uint8_t* dataPtr, size_t dataLen); + GenericDnsResourceData(const uint8_t* dataPtr, size_t dataLen); /** * A c'tor for this class diff --git a/Packet++/src/DnsLayer.cpp b/Packet++/src/DnsLayer.cpp index 644ea89b7b..3863c4a7b8 100644 --- a/Packet++/src/DnsLayer.cpp +++ b/Packet++/src/DnsLayer.cpp @@ -475,7 +475,7 @@ DnsResource* DnsLayer::addResource(DnsResourceType resType, const std::string& n uint32_t ttl, IDnsResourceData* data) { // create new query on temporary buffer - uint8_t newResourceRawData[256]; + uint8_t newResourceRawData[4096]; memset(newResourceRawData, 0, sizeof(newResourceRawData)); DnsResource* newResource = new DnsResource(newResourceRawData, resType); diff --git a/Packet++/src/DnsResource.cpp b/Packet++/src/DnsResource.cpp index 8c72cc578e..fec1bf0597 100644 --- a/Packet++/src/DnsResource.cpp +++ b/Packet++/src/DnsResource.cpp @@ -12,7 +12,7 @@ namespace pcpp IDnsResource::IDnsResource(DnsLayer* dnsLayer, size_t offsetInLayer) : m_DnsLayer(dnsLayer), m_OffsetInLayer(offsetInLayer), m_NextResource(nullptr) { - char decodedName[256]; + char decodedName[4096]; m_NameLength = decodeName((const char*)getRawData(), decodedName); if (m_NameLength > 0) m_DecodedName = decodedName; @@ -83,8 +83,8 @@ size_t IDnsResource::decodeName(const char* encodedName, char* result, int itera return 0; } - char tempResult[256]; - memset(tempResult, 0, 256); + char tempResult[4096]; + memset(tempResult, 0, sizeof(tempResult)); int i = 0; decodeName((const char*)(m_DnsLayer->m_Data + offsetInLayer), tempResult, iteration+1); while (tempResult[i] != 0 && decodedNameLength < 255) @@ -224,7 +224,7 @@ void IDnsResource::setDnsClass(DnsClass newClass) bool IDnsResource::setName(const std::string& newName) { - char encodedName[256]; + char encodedName[4096]; size_t encodedNameLen = 0; encodeName(newName, encodedName, encodedNameLen); if (m_DnsLayer != nullptr) @@ -345,7 +345,7 @@ bool DnsResource::setData(IDnsResourceData* data) { // convert data to byte array according to the DNS type size_t dataLength = 0; - uint8_t dataAsByteArr[256]; + uint8_t dataAsByteArr[4096]; if (data == nullptr) { diff --git a/Packet++/src/DnsResourceData.cpp b/Packet++/src/DnsResourceData.cpp index 8183511d27..88b73b373c 100644 --- a/Packet++/src/DnsResourceData.cpp +++ b/Packet++/src/DnsResourceData.cpp @@ -38,7 +38,7 @@ StringDnsResourceData::StringDnsResourceData(const uint8_t* dataPtr, size_t data { if (dataPtr && dataLen > 0) { - char tempResult[256]; + char tempResult[4096]; decodeName((const char*)dataPtr, tempResult, dnsResource); m_Data = tempResult; } @@ -106,7 +106,7 @@ MxDnsResourceData::MxDnsResourceData(uint8_t* dataPtr, size_t dataLen, IDnsResou if (dataPtr && dataLen > 0) { uint16_t preference = be16toh(*(uint16_t*)dataPtr); - char tempMX[256]; + char tempMX[4096]; decodeName((const char*)(dataPtr + sizeof(preference)), tempMX, dnsResource); m_Data.preference = preference; m_Data.mailExchange = tempMX; @@ -150,7 +150,7 @@ bool MxDnsResourceData::toByteArr(uint8_t* arr, size_t& arrLength, IDnsResource* return true; } -GenericDnsResourceData::GenericDnsResourceData(uint8_t* dataPtr, size_t dataLen) +GenericDnsResourceData::GenericDnsResourceData(const uint8_t* dataPtr, size_t dataLen) { m_Data = nullptr; m_DataLen = 0; @@ -218,7 +218,7 @@ bool GenericDnsResourceData::toByteArr(uint8_t* arr, size_t& arrLength, IDnsReso { if (m_DataLen == 0 || m_Data == nullptr) { - PCPP_LOG_ERROR("Input data is null or illegal"); + PCPP_LOG_ERROR("Input data is null or illegal" << "|m_DataLen:" << m_DataLen); return false; } diff --git a/Tests/Packet++Test/TestDefinition.h b/Tests/Packet++Test/TestDefinition.h index 02c5574a4e..d4c1ee8571 100644 --- a/Tests/Packet++Test/TestDefinition.h +++ b/Tests/Packet++Test/TestDefinition.h @@ -84,6 +84,7 @@ PTF_TEST_CASE(DnsLayerEditTest); PTF_TEST_CASE(DnsLayerRemoveResourceTest); PTF_TEST_CASE(DnsOverTcpParsingTest); PTF_TEST_CASE(DnsOverTcpCreationTest); +PTF_TEST_CASE(DnsLayerAddDnsKeyTest); // Implemented in IcmpTests.cpp PTF_TEST_CASE(IcmpParsingTest); diff --git a/Tests/Packet++Test/Tests/DnsTests.cpp b/Tests/Packet++Test/Tests/DnsTests.cpp index 4446e5a2ac..2dae844055 100644 --- a/Tests/Packet++Test/Tests/DnsTests.cpp +++ b/Tests/Packet++Test/Tests/DnsTests.cpp @@ -473,7 +473,19 @@ PTF_TEST_CASE(DnsLayerResourceCreationTest) PTF_ASSERT_BUF_COMPARE(dnsEdit7Packet.getRawPacket()->getRawData(), buffer7, bufferLength7); } // DnsLayerResourceCreationTest +PTF_TEST_CASE(DnsLayerAddDnsKeyTest) +{ + + // data length overflow 256 + const std::string dnskey = "AwEAAaz/tAm8yTn4Mfeh5eyI96WSVexTBAvkMgJzkKTOiW1vkIbzxeF3+/4RgWOq7HrxRixHlFlExOLAJr5emLvN7SWXgnLh4+B5xQ \ +lNVz8Og8kvArMtNROxVQuCaSnIDdD5LKyWbRd2n9WGe2R8PzgCmr3EgVLrjyBxWezF0jLHwVN8efS3rCj/EWgvIWgb9tarpVUDK/b58Da+sqqls3eNbuv7pr+eoZG+Sr \ +DK6nWeL3c6H5Apxz7LjVc1uTIdsIXxuOLYA4/ilBmSVIzuDWfdRUfhHdY6+cn8HFRm+2hM8AnXGXws9555KrUB5qihylGa8subX2Nn6UwNR1AkUTV74bU="; + pcpp::DnsLayer dnsLayer; + pcpp::GenericDnsResourceData genericData(reinterpret_cast(dnskey.c_str()), dnskey.size()); + const auto* additional = dnsLayer.addAnswer("github.com", pcpp::DNS_TYPE_DNSKEY, pcpp::DNS_CLASS_IN, 32, &genericData); + PTF_ASSERT_NOT_NULL(additional); +} PTF_TEST_CASE(DnsLayerEditTest) { diff --git a/Tests/Packet++Test/main.cpp b/Tests/Packet++Test/main.cpp index 93a9be987b..731bf31fdd 100644 --- a/Tests/Packet++Test/main.cpp +++ b/Tests/Packet++Test/main.cpp @@ -182,6 +182,7 @@ int main(int argc, char* argv[]) PTF_RUN_TEST(DnsLayerRemoveResourceTest, "dns"); PTF_RUN_TEST(DnsOverTcpParsingTest, "dns"); PTF_RUN_TEST(DnsOverTcpCreationTest, "dns"); + PTF_RUN_TEST(DnsLayerAddDnsKeyTest, "dns"); PTF_RUN_TEST(IcmpParsingTest, "icmp"); PTF_RUN_TEST(IcmpCreationTest, "icmp");