From 9bd1fe86eed554d18f81b3433ad8459bd1cf62a6 Mon Sep 17 00:00:00 2001 From: root Date: Tue, 4 Mar 2025 07:17:23 -0800 Subject: [PATCH 1/5] Test commit: added testfile.txt --- testfile.txt | 1 + 1 file changed, 1 insertion(+) create mode 100644 testfile.txt diff --git a/testfile.txt b/testfile.txt new file mode 100644 index 000000000..9f4b6d8bf --- /dev/null +++ b/testfile.txt @@ -0,0 +1 @@ +This is a test file From 38a5e08af246d277017e43f81aff7a22be23bd01 Mon Sep 17 00:00:00 2001 From: hayden Date: Tue, 4 Mar 2025 08:18:23 -0800 Subject: [PATCH 2/5] Removed test file --- testfile.txt | 1 - 1 file changed, 1 deletion(-) delete mode 100644 testfile.txt diff --git a/testfile.txt b/testfile.txt deleted file mode 100644 index 9f4b6d8bf..000000000 --- a/testfile.txt +++ /dev/null @@ -1 +0,0 @@ -This is a test file From 862abfe75d39012e8e1d1e4c437add32859855a0 Mon Sep 17 00:00:00 2001 From: "hayden.carroll-NS1" Date: Wed, 5 Mar 2025 07:51:55 -0800 Subject: [PATCH 3/5] Enhance decodeName() with additional bounds checks and error logging --- libs/visor_dns/DnsResource.cpp | 54 +++++++++++++++------------------- 1 file changed, 24 insertions(+), 30 deletions(-) diff --git a/libs/visor_dns/DnsResource.cpp b/libs/visor_dns/DnsResource.cpp index 97ad533c9..e82e80cb1 100644 --- a/libs/visor_dns/DnsResource.cpp +++ b/libs/visor_dns/DnsResource.cpp @@ -57,12 +57,14 @@ size_t IDnsResource::decodeName(const char *encodedName, char *result, int itera char *resultPtr = result; resultPtr[0] = 0; - size_t curOffsetInLayer = (uint8_t *)encodedName - m_DnsLayer->m_Data; - if (curOffsetInLayer + 1 > m_DnsLayer->m_DataLen) - return encodedNameLength; + if (curOffsetInLayer >= m_DnsLayer->m_DataLen) { + PCPP_LOG_ERROR("decodeName: curOffsetInLayer (%zu) >= m_DnsLayer->m_DataLen (%zu)", curOffsetInLayer, m_DnsLayer->m_DataLen); + return 0; + } if (iteration > 20) { - return encodedNameLength; + PCPP_LOG_ERROR("decodeName: recursion depth (%d) exceeded limit", iteration); + return 0; } uint8_t wordLength = encodedName[0]; @@ -71,17 +73,25 @@ size_t IDnsResource::decodeName(const char *encodedName, char *result, int itera while (wordLength != 0) { // A pointer to another place in the packet if ((wordLength & 0xc0) == 0xc0) { - if (curOffsetInLayer + 2 > m_DnsLayer->m_DataLen || encodedNameLength > 255) - return encodedNameLength; + if (curOffsetInLayer + 2 > m_DnsLayer->m_DataLen || encodedNameLength >= 255) { + PCPP_LOG_ERROR("decodeName pointer case: out-of-bounds: curOffsetInLayer (%zu) + 2 > m_DnsLayer->m_DataLen (%zu) or encodedNameLength (%zu) >= 255", curOffsetInLayer, m_DnsLayer->m_DataLen, encodedNameLength); + return 0; + } uint16_t offsetInLayer = (wordLength & 0x3f) * 256 + (0xFF & encodedName[1]); if (offsetInLayer < sizeof(dnshdr) || offsetInLayer >= m_DnsLayer->m_DataLen) { - PCPP_LOG_ERROR("DNS parsing error: name pointer is illegal"); + PCPP_LOG_ERROR("DNS parsing error: name pointer is illegal, offsetInLayer = %u", offsetInLayer); return 0; } char tempResult[256]; memset(tempResult, 0, 256); + size_t ret = decodeName((const char *)(m_DnsLayer->m_Data + offsetInLayer), tempResult, iteration + 1); + if (ret == 0) { + PCPP_LOG_ERROR("decodeName: recursive call failed at offset %u", offsetInLayer); + return 0; + } + int i = 0; decodeName((const char *)(m_DnsLayer->m_Data + offsetInLayer), tempResult, iteration + 1); while (tempResult[i] != 0 && decodedNameLength < 255) { @@ -93,20 +103,12 @@ size_t IDnsResource::decodeName(const char *encodedName, char *result, int itera resultPtr[0] = 0; // in this case the length of the pointer is: 1 byte for 0xc0 + 1 byte for the offset itself - return encodedNameLength + sizeof(uint16_t); + return encodedNameLength + sizeof(uint16_t); // note - just hard code to 2 for simplicity? } else { // return if next word would be outside of the DNS layer or overflow the buffer behind resultPtr - if (curOffsetInLayer + wordLength + 1 > m_DnsLayer->m_DataLen || encodedNameLength + wordLength > 255) { - // add the last '\0' to the decoded string - if (encodedNameLength == 256) { - resultPtr--; - decodedNameLength--; - } else { - encodedNameLength++; - } - - resultPtr[0] = 0; - return encodedNameLength; + if (curOffsetInLayer + wordLength + 1 > m_DnsLayer->m_DataLen || encodedNameLength + wordLength => 255) { + PCPP_LOG_ERROR("decodeName: label out-of-bounds: curOffsetInLayer (%zu) + wordLength (%u) + 1 > m_DnsLayer->m_DataLen (%zu) or encodedNameLength (%zu) + wordLength (%u) >= 255",curOffsetInLayer, wordLength, m_DnsLayer->m_DataLen, encodedNameLength, wordLength); + return 0; } memcpy(resultPtr, encodedName + 1, wordLength); @@ -118,17 +120,9 @@ size_t IDnsResource::decodeName(const char *encodedName, char *result, int itera encodedNameLength += wordLength + 1; curOffsetInLayer = (uint8_t *)encodedName - m_DnsLayer->m_Data; - if (curOffsetInLayer + 1 > m_DnsLayer->m_DataLen) { - // add the last '\0' to the decoded string - if (encodedNameLength == 256) { - decodedNameLength--; - resultPtr--; - } else { - encodedNameLength++; - } - - resultPtr[0] = 0; - return encodedNameLength; + if (curOffsetInLayer >= m_DnsLayer->m_DataLen) { + PCPP_LOG_ERROR("decodeName: after label, curOffsetInLayer (%zu) >= m_DnsLayer->m_DataLen (%zu)", curOffsetInLayer, m_DnsLayer->m_DataLen); + return 0; } wordLength = encodedName[0]; From 00efa2712438d56c599c17a8096c58e95abec31f Mon Sep 17 00:00:00 2001 From: "hayden.carroll-NS1" Date: Wed, 5 Mar 2025 07:54:29 -0800 Subject: [PATCH 4/5] deleted line by mistake --- libs/visor_dns/DnsResource.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/libs/visor_dns/DnsResource.cpp b/libs/visor_dns/DnsResource.cpp index e82e80cb1..28e7fcc24 100644 --- a/libs/visor_dns/DnsResource.cpp +++ b/libs/visor_dns/DnsResource.cpp @@ -57,6 +57,7 @@ size_t IDnsResource::decodeName(const char *encodedName, char *result, int itera char *resultPtr = result; resultPtr[0] = 0; + size_t curOffsetInLayer = (uint8_t *)encodedName - m_DnsLayer->m_Data; if (curOffsetInLayer >= m_DnsLayer->m_DataLen) { PCPP_LOG_ERROR("decodeName: curOffsetInLayer (%zu) >= m_DnsLayer->m_DataLen (%zu)", curOffsetInLayer, m_DnsLayer->m_DataLen); return 0; From 6cb6b42f94dbd7ae8d3864391949c08c8033c977 Mon Sep 17 00:00:00 2001 From: "hayden.carroll-NS1" Date: Thu, 6 Mar 2025 03:10:28 -0800 Subject: [PATCH 5/5] hanged the logs to fmt format for compatibility with the PCPP_LOG_ERROR macro --- libs/visor_dns/DnsResource.cpp | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/libs/visor_dns/DnsResource.cpp b/libs/visor_dns/DnsResource.cpp index 28e7fcc24..cd35ed230 100644 --- a/libs/visor_dns/DnsResource.cpp +++ b/libs/visor_dns/DnsResource.cpp @@ -59,12 +59,12 @@ size_t IDnsResource::decodeName(const char *encodedName, char *result, int itera size_t curOffsetInLayer = (uint8_t *)encodedName - m_DnsLayer->m_Data; if (curOffsetInLayer >= m_DnsLayer->m_DataLen) { - PCPP_LOG_ERROR("decodeName: curOffsetInLayer (%zu) >= m_DnsLayer->m_DataLen (%zu)", curOffsetInLayer, m_DnsLayer->m_DataLen); + PCPP_LOG_ERROR(fmt::format("decodeName: curOffsetInLayer ({}) >= m_DnsLayer->m_DataLen ({})", curOffsetInLayer, m_DnsLayer->m_DataLen).c_str()); return 0; } if (iteration > 20) { - PCPP_LOG_ERROR("decodeName: recursion depth (%d) exceeded limit", iteration); + PCPP_LOG_ERROR(fmt::format("decodeName: recursion depth ({}) exceeded limit", iteration).c_str()); return 0; } @@ -75,13 +75,13 @@ size_t IDnsResource::decodeName(const char *encodedName, char *result, int itera // A pointer to another place in the packet if ((wordLength & 0xc0) == 0xc0) { if (curOffsetInLayer + 2 > m_DnsLayer->m_DataLen || encodedNameLength >= 255) { - PCPP_LOG_ERROR("decodeName pointer case: out-of-bounds: curOffsetInLayer (%zu) + 2 > m_DnsLayer->m_DataLen (%zu) or encodedNameLength (%zu) >= 255", curOffsetInLayer, m_DnsLayer->m_DataLen, encodedNameLength); + PCPP_LOG_ERROR(fmt::format("decodeName pointer case: out-of-bounds: curOffsetInLayer ({}) + 2 > m_DnsLayer->m_DataLen ({}) or encodedNameLength ({}) >= 255",curOffsetInLayer, m_DnsLayer->m_DataLen, encodedNameLength).c_str()); return 0; } uint16_t offsetInLayer = (wordLength & 0x3f) * 256 + (0xFF & encodedName[1]); if (offsetInLayer < sizeof(dnshdr) || offsetInLayer >= m_DnsLayer->m_DataLen) { - PCPP_LOG_ERROR("DNS parsing error: name pointer is illegal, offsetInLayer = %u", offsetInLayer); + PCPP_LOG_ERROR(fmt::format("DNS parsing error: name pointer is illegal, offsetInLayer = {}", offsetInLayer).c_str()); return 0; } @@ -89,12 +89,11 @@ size_t IDnsResource::decodeName(const char *encodedName, char *result, int itera memset(tempResult, 0, 256); size_t ret = decodeName((const char *)(m_DnsLayer->m_Data + offsetInLayer), tempResult, iteration + 1); if (ret == 0) { - PCPP_LOG_ERROR("decodeName: recursive call failed at offset %u", offsetInLayer); + PCPP_LOG_ERROR(fmt::format("decodeName: recursive call failed at offset {}", offsetInLayer).c_str()); return 0; } int i = 0; - decodeName((const char *)(m_DnsLayer->m_Data + offsetInLayer), tempResult, iteration + 1); while (tempResult[i] != 0 && decodedNameLength < 255) { resultPtr[0] = tempResult[i++]; resultPtr++; @@ -104,11 +103,12 @@ size_t IDnsResource::decodeName(const char *encodedName, char *result, int itera resultPtr[0] = 0; // in this case the length of the pointer is: 1 byte for 0xc0 + 1 byte for the offset itself - return encodedNameLength + sizeof(uint16_t); // note - just hard code to 2 for simplicity? + return encodedNameLength + 2; } else { // return if next word would be outside of the DNS layer or overflow the buffer behind resultPtr - if (curOffsetInLayer + wordLength + 1 > m_DnsLayer->m_DataLen || encodedNameLength + wordLength => 255) { - PCPP_LOG_ERROR("decodeName: label out-of-bounds: curOffsetInLayer (%zu) + wordLength (%u) + 1 > m_DnsLayer->m_DataLen (%zu) or encodedNameLength (%zu) + wordLength (%u) >= 255",curOffsetInLayer, wordLength, m_DnsLayer->m_DataLen, encodedNameLength, wordLength); + if (curOffsetInLayer + wordLength + 1 > m_DnsLayer->m_DataLen || encodedNameLength + wordLength >= 255) { + PCPP_LOG_ERROR(fmt::format("decodeName: label out-of-bounds: curOffsetInLayer ({}) + wordLength ({}) + 1 > m_DnsLayer->m_DataLen ({}) or encodedNameLength ({}) + wordLength ({}) >= 255", + curOffsetInLayer, wordLength, m_DnsLayer->m_DataLen, encodedNameLength, wordLength).c_str()); return 0; } @@ -122,7 +122,8 @@ size_t IDnsResource::decodeName(const char *encodedName, char *result, int itera curOffsetInLayer = (uint8_t *)encodedName - m_DnsLayer->m_Data; if (curOffsetInLayer >= m_DnsLayer->m_DataLen) { - PCPP_LOG_ERROR("decodeName: after label, curOffsetInLayer (%zu) >= m_DnsLayer->m_DataLen (%zu)", curOffsetInLayer, m_DnsLayer->m_DataLen); + PCPP_LOG_ERROR(fmt::format("decodeName: after label, curOffsetInLayer ({}) >= m_DnsLayer->m_DataLen ({})", + curOffsetInLayer, m_DnsLayer->m_DataLen).c_str()); return 0; } @@ -142,6 +143,7 @@ size_t IDnsResource::decodeName(const char *encodedName, char *result, int itera return encodedNameLength; } + void IDnsResource::encodeName(const std::string &decodedName, char *result, size_t &resultLen) { resultLen = 0;