]> granicus.if.org Git - pdns/commitdiff
dnsreplay: Add more checks against bogus PCAP
authorRemi Gacogne <remi.gacogne@powerdns.com>
Tue, 5 Dec 2017 15:31:56 +0000 (16:31 +0100)
committerRemi Gacogne <remi.gacogne@powerdns.com>
Fri, 11 May 2018 08:21:52 +0000 (10:21 +0200)
pdns/dnspcap.cc
pdns/dnspcap.hh
pdns/dnsreplay.cc

index 4104b539a759a720d547598724ba941e1d34826d..44a720b35408bf878e18a7a06443b2818a7c0702 100644 (file)
@@ -97,27 +97,54 @@ try
       continue;
     }
 
-    d_ether=reinterpret_cast<struct ether_header*>(d_buffer);
-    d_lcc=reinterpret_cast<struct pdns_lcc_header*>(d_buffer);
+    if (d_pheader.caplen < d_skipMediaHeader) {
+      d_runts++;
+      continue;
+    }
 
     d_ip=reinterpret_cast<struct ip*>(d_buffer + d_skipMediaHeader);
     d_ip6=reinterpret_cast<struct ip6_hdr*>(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<struct ether_header*>(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<struct pdns_lcc_header*>(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<const struct udphdr*>(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<const struct udphdr*>(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;
index 27e78ad9d213734d0f6202087d5619946759836b..0eade284e7203b6d72ce4052952b8cf7db9fa45d 100644 (file)
@@ -123,7 +123,7 @@ public:
 private:
   FILE* d_fp;
   string d_fname;
-  int d_skipMediaHeader;
+  unsigned int d_skipMediaHeader;
 };
 
 class PcapPacketWriter
index 99ddaffad4edfe3817746c7c6b47e97641f868b1..44238f7725f82d7ae6a82ff456e4bab859900b80 100644 (file)
@@ -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<uint16_t>::max()) {
+    /* too large for an DNS UDP query, something is not right */
+    return false;
+  }
+  dnsheader* dh=const_cast<dnsheader*>(reinterpret_cast<const dnsheader*>(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;