From: Remi Gacogne Date: Tue, 5 Dec 2017 15:31:56 +0000 (+0100) Subject: dnsreplay: Add more checks against bogus PCAP X-Git-Tag: dnsdist-1.3.1~97^2 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=fe92c902aeb4e12778db8029db241b637fa2e548;p=pdns dnsreplay: Add more checks against bogus PCAP --- diff --git a/pdns/dnspcap.cc b/pdns/dnspcap.cc index 4104b539a..44a720b35 100644 --- a/pdns/dnspcap.cc +++ b/pdns/dnspcap.cc @@ -97,27 +97,54 @@ try continue; } - d_ether=reinterpret_cast(d_buffer); - d_lcc=reinterpret_cast(d_buffer); + if (d_pheader.caplen < d_skipMediaHeader) { + d_runts++; + continue; + } d_ip=reinterpret_cast(d_buffer + d_skipMediaHeader); d_ip6=reinterpret_cast(d_buffer + d_skipMediaHeader); uint16_t contentCode=0; - if(d_pfh.linktype==1) + + if(d_pfh.linktype==1) { + if (d_pheader.caplen < sizeof(*d_ether)) { + d_runts++; + continue; + } + d_ether=reinterpret_cast(d_buffer); contentCode=ntohs(d_ether->ether_type); + } else if(d_pfh.linktype==101) { + if (d_pheader.caplen < (d_skipMediaHeader + sizeof(*d_ip))) { + d_runts++; + continue; + } if(d_ip->ip_v==4) contentCode = 0x0800; else contentCode = 0x86dd; } - else if(d_pfh.linktype==113) + else if(d_pfh.linktype==113) { + if (d_pheader.caplen < sizeof(*d_lcc)) { + d_runts++; + continue; + } + d_lcc=reinterpret_cast(d_buffer); contentCode=ntohs(d_lcc->lcc_protocol); + } if(contentCode==0x0800 && d_ip->ip_p==17) { // udp + if (d_pheader.caplen < (d_skipMediaHeader + (4 * d_ip->ip_hl) + sizeof(*d_udp))) { + d_runts++; + continue; + } d_udp=reinterpret_cast(d_buffer + d_skipMediaHeader + 4 * d_ip->ip_hl); d_payload = (unsigned char*)d_udp + sizeof(struct udphdr); d_len = ntohs(d_udp->uh_ulen) - sizeof(struct udphdr); + if (d_pheader.caplen < (d_skipMediaHeader + (4 * d_ip->ip_hl) + sizeof(*d_udp) + d_len)) { + d_runts++; + continue; + } if((const char*)d_payload + d_len > d_buffer + d_pheader.caplen) { d_runts++; continue; @@ -125,10 +152,18 @@ try d_correctpackets++; return true; } - if(contentCode==0x86dd && d_ip6->ip6_ctlun.ip6_un1.ip6_un1_nxt==17) { // udpv6, we ignore anything with extension hdr + else if(contentCode==0x86dd && d_ip6->ip6_ctlun.ip6_un1.ip6_un1_nxt==17) { // udpv6, we ignore anything with extension hdr + if (d_pheader.caplen < (d_skipMediaHeader + sizeof(struct ip6_hdr) + sizeof(struct udphdr))) { + d_runts++; + continue; + } d_udp=reinterpret_cast(d_buffer + d_skipMediaHeader + sizeof(struct ip6_hdr)); d_payload = (unsigned char*)d_udp + sizeof(struct udphdr); d_len = ntohs(d_udp->uh_ulen) - sizeof(struct udphdr); + if (d_pheader.caplen < (d_skipMediaHeader + sizeof(struct ip6_hdr) + sizeof(struct udphdr) + d_len)) { + d_runts++; + continue; + } if((const char*)d_payload + d_len > d_buffer + d_pheader.caplen) { d_runts++; continue; diff --git a/pdns/dnspcap.hh b/pdns/dnspcap.hh index 27e78ad9d..0eade284e 100644 --- a/pdns/dnspcap.hh +++ b/pdns/dnspcap.hh @@ -123,7 +123,7 @@ public: private: FILE* d_fp; string d_fname; - int d_skipMediaHeader; + unsigned int d_skipMediaHeader; }; class PcapPacketWriter diff --git a/pdns/dnsreplay.cc b/pdns/dnsreplay.cc index 99ddaffad..44238f772 100644 --- a/pdns/dnsreplay.cc +++ b/pdns/dnsreplay.cc @@ -569,7 +569,7 @@ static void generateOptRR(const std::string& optRData, string& res) res.append(optRData.c_str(), optRData.length()); } -static void addECSOption(char* packet, const size_t& packetSize, uint16_t* len, const ComboAddress& remote, int stamp) +static void addECSOption(char* packet, const size_t packetSize, uint16_t* len, const ComboAddress& remote, int stamp) { string EDNSRR; struct dnsheader* dh = (struct dnsheader*) packet; @@ -590,7 +590,7 @@ static void addECSOption(char* packet, const size_t& packetSize, uint16_t* len, uint16_t arcount = ntohs(dh->arcount); /* does it fit in the existing buffer? */ - if (packetSize > *len && packetSize - *len > EDNSRR.size()) { + if (packetSize > *len && (packetSize - *len) > EDNSRR.size()) { arcount++; dh->arcount = htons(arcount); memcpy(packet + *len, EDNSRR.c_str(), EDNSRR.size()); @@ -603,9 +603,16 @@ static uint16_t g_pcapDnsPort; static bool sendPacketFromPR(PcapPacketReader& pr, const ComboAddress& remote, int stamp) { - dnsheader* dh=(dnsheader*)pr.d_payload; bool sent=false; - if((ntohs(pr.d_udp->uh_dport)!=g_pcapDnsPort && ntohs(pr.d_udp->uh_sport)!=g_pcapDnsPort) || dh->rd != g_rdSelector || (unsigned int)pr.d_len <= sizeof(dnsheader)) + if (pr.d_len <= sizeof(dnsheader)) { + return sent; + } + if (pr.d_len > std::numeric_limits::max()) { + /* too large for an DNS UDP query, something is not right */ + return false; + } + dnsheader* dh=const_cast(reinterpret_cast(pr.d_payload)); + if((ntohs(pr.d_udp->uh_dport)!=g_pcapDnsPort && ntohs(pr.d_udp->uh_sport)!=g_pcapDnsPort) || dh->rd != g_rdSelector) return sent; QuestionData qd; @@ -619,8 +626,16 @@ static bool sendPacketFromPR(PcapPacketReader& pr, const ComboAddress& remote, i // dh->rd=1; // useful to replay traffic to auths to a recursor uint16_t dlen = pr.d_len; - if (stamp >= 0) addECSOption((char*)pr.d_payload, 1500, &dlen, pr.getSource(), stamp); - pr.d_len=dlen; + if (stamp >= 0) { + static_assert(sizeof(pr.d_buffer) >= 1500, "The size of the underlying buffer should be at least 1500 bytes"); + if (dlen > 1500) { + /* the existing packet is larger than the maximum size we are willing to send, and it won't get better by adding ECS */ + return false; + } + addECSOption((char*)pr.d_payload, 1500, &dlen, pr.getSource(), stamp); + pr.d_len=dlen; + } + s_socket->sendTo((const char*)pr.d_payload, dlen, remote); sent=true; dh->id=tmp;