seladb / PcapPlusPlus

PcapPlusPlus is a multiplatform C++ library for capturing, parsing and crafting of network packets. It is designed to be efficient, powerful and easy to use. It provides C++ wrappers for the most popular packet processing engines such as libpcap, Npcap, WinPcap, DPDK, AF_XDP and PF_RING.
https://pcapplusplus.github.io/
The Unlicense
2.71k stars 659 forks source link

DNS parser crashes on malicious dns packet #508

Closed xmkg closed 4 years ago

xmkg commented 4 years ago

I'm currently experimenting with DNS vulnerabilities, and using pcapplusplus (v19.12) to read pcap files & individual dns payloads. It appears the pcap file provided in this issue breaks the internal dns parser provided with pcapplusplus.

The pcap file can be found in sample captures page of official wireshark wiki page. ("zlip-3.pcap" in crack traces section)

https://wiki.wireshark.org/SampleCaptures?action=AttachFile&do=get&target=zlip-3.pcap

Crash happens in DnsResource.cpp, line 101.


/*...*/
// A string to parse
while (wordLength != 0)
{
    // A pointer to another place in the packet
    if ((wordLength & 0xc0) == 0xc0)
    {
        if (curOffsetInLayer + 2 > m_DnsLayer->m_DataLen)
            return encodedNameLength;

        uint16_t offsetInLayer = (wordLength & 0x3f)*256 + (0xFF & encodedName[1]);
        if (offsetInLayer < sizeof(dnshdr) || offsetInLayer >= m_DnsLayer->m_DataLen)
        {
            LOG_ERROR("DNS parsing error: name pointer is illegal");
            return 0;
        }

        char tempResult[256];
        int i = 0;
        decodeName((const char*)(m_DnsLayer->m_Data + offsetInLayer), tempResult, iteration+1);
        while (tempResult[i] != 0)
        {
            resultPtr[0] = tempResult[i++];
            resultPtr++;
        }

        resultPtr[0] = 0;

        // in this case the length of the pointer is: 1B for 0xc0 + 1B for the offset itself
        return encodedNameLength + sizeof(uint16_t);
    }
    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--;
            else
                encodedNameLength++;

            resultPtr[0] = 0;   
            return encodedNameLength;
        }

        memcpy(resultPtr, encodedName+1, wordLength); // <---- CRASHES HERE
        resultPtr += wordLength;
        resultPtr[0] = '.';
        resultPtr++;
        encodedName += wordLength + 1;
        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)
                resultPtr--;
            else
                encodedNameLength++;

            resultPtr[0] = 0;   
            return encodedNameLength;
        }

        wordLength = encodedName[0];
    }
}
/* ... */
seladb commented 4 years ago

I fixed a lot of issues with DNS parsing since v19.12. Can you please pull the latest code from master and test again?

xmkg commented 4 years ago

Tested with the current master branch commit (6a78b5da5b5dbc31c4ccc0ff74513580a149c71e), it seems this issue is fixed now. I will re-test when the official release arrives. Closing this now.

xmkg commented 4 years ago

btw, @seladb is there a way to disable dns parsing (or any other L7 protocol, actually) programatically?

seladb commented 4 years ago

Yes, the way to do it is to use the parseUntil (for specific protocol) or parseUntilLayer (for overall L7) parameters in pcpp::Packet c'tor: https://pcapplusplus.github.io/api-docs/classpcpp_1_1_packet.html#aaf61c3f9a5c12a7047774b42f31ee2ec

Here are some examples from PcapPlusPlus tests: https://github.com/seladb/PcapPlusPlus/blob/6a78b5da5b5dbc31c4ccc0ff74513580a149c71e/Tests/Packet%2B%2BTest/Tests/PacketTests.cpp#L592

xmkg commented 4 years ago

Yes, the way to do it is to use the parseUntil (for specific protocol) or parseUntilLayer (for overall L7) parameters in pcpp::Packet c'tor: https://pcapplusplus.github.io/api-docs/classpcpp_1_1_packet.html#aaf61c3f9a5c12a7047774b42f31ee2ec

Here are some examples from PcapPlusPlus tests: https://github.com/seladb/PcapPlusPlus/blob/6a78b5da5b5dbc31c4ccc0ff74513580a149c71e/Tests/Packet%2B%2BTest/Tests/PacketTests.cpp#L592

Great, thanks!