Skip to content

Fix decode name #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
haydencarroll-NS1 merged 5 commits into from
Mar 10, 2025
Merged

Fix decode name #2

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
9bd1fe8
Test commit: added testfile.txt
Mar 4, 2025
38a5e08
Removed test file
Mar 4, 2025
862abfe
Enhance decodeName() with additional bounds checks and error logging
haydencarroll-NS1 Mar 5, 2025
00efa27
deleted line by mistake
haydencarroll-NS1 Mar 5, 2025
6cb6b42
hanged the logs to fmt format for compatibility with the PCPP_LOG_ERR…
haydencarroll-NS1 Mar 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 27 additions & 30 deletions libs/visor_dns/DnsResource.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,14 @@ size_t IDnsResource::decodeName(const char *encodedName, char *result, int itera
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(fmt::format("decodeName: curOffsetInLayer ({}) >= m_DnsLayer->m_DataLen ({})", curOffsetInLayer, m_DnsLayer->m_DataLen).c_str());
return 0;
}

if (iteration > 20) {
return encodedNameLength;
PCPP_LOG_ERROR(fmt::format("decodeName: recursion depth ({}) exceeded limit", iteration).c_str());
return 0;
}

uint8_t wordLength = encodedName[0];
Expand All @@ -71,19 +74,26 @@ 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(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");
PCPP_LOG_ERROR(fmt::format("DNS parsing error: name pointer is illegal, offsetInLayer = {}", offsetInLayer).c_str());
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(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++;
Expand All @@ -93,20 +103,13 @@ 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 + 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) {
// 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(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;
}

memcpy(resultPtr, encodedName + 1, wordLength);
Expand All @@ -118,17 +121,10 @@ 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(fmt::format("decodeName: after label, curOffsetInLayer ({}) >= m_DnsLayer->m_DataLen ({})",
curOffsetInLayer, m_DnsLayer->m_DataLen).c_str());
return 0;
}

wordLength = encodedName[0];
Expand All @@ -147,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;
Expand Down