Check destination of DNS UDP query replies.

At any time, dnsmasq will have a set of sockets open, bound to
random ports, on which it sends queries to upstream nameservers.
This patch fixes the existing problem that a reply for ANY in-flight
query would be accepted via ANY open port, which increases the
chances of an attacker flooding answers "in the blind" in an
attempt to poison the DNS cache. CERT VU#434904 refers.

Backported for CVE-2020-25684

Change-Id: I11790b18ad6e179a6f3f47fee310cd00ab3c7cdd

diff --git a/CHANGELOG b/CHANGELOG
index 470926d..c1bab72 100644 (file)
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -3,6 +3,11 @@ Backpored patch
        dnsmasq with DNSSEC compiled in and enabled is vulnerable to this,
        referenced by CERT VU#434904.
 
+       Be sure to only accept UDP DNS query replies at the address
+       from which the query was originated. This keeps as much entropy
+       in the {query-ID, random-port} tuple as possible, help defeat
+       cache poisoning attacks. Refer: CERT VU#434904.
+
 version 2.79
        Fix parsing of CNAME arguments, which are confused by extra spaces.
        Thanks to Diego Aguirre for spotting the bug.

diff --git a/src/forward.c b/src/forward.c
index cdd11d3..a65acea 100644 (file)
--- a/src/forward.c
+++ b/src/forward.c
@@ -16,7 +16,7 @@
 
 #include "dnsmasq.h"
 
-static struct frec *lookup_frec(unsigned short id, void *hash);
+static struct frec *lookup_frec(unsigned short id, int fd, int family, void *hash);
 static struct frec *lookup_frec_by_sender(unsigned short id,
                                          union mysockaddr *addr,
                                          void *hash);
@@ -779,8 +779,7 @@ void reply_query(int fd, int family, time_t now)
   hash = &crc;
   crc = questions_crc(header, n, daemon->namebuff);
 #endif
-  
-  if (!(forward = lookup_frec(ntohs(header->id), hash)))
+  if (!(forward = lookup_frec(ntohs(header->id), fd, family, hash)))
     return;
   
   /* log_query gets called indirectly all over the place, so 
@@ -2195,15 +2194,26 @@ struct frec *get_new_frec(time_t now, int *wait, int force)
 }
 
 /* crc is all-ones if not known. */
-static struct frec *lookup_frec(unsigned short id, void *hash)
+static struct frec *lookup_frec(unsigned short id, int fd, int family, void *hash)
 {
   struct frec *f;
 
   for(f = daemon->frec_list; f; f = f->next)
     if (f->sentto && f->new_id == id && 
-       (!hash || memcmp(hash, f->hash, HASH_SIZE) == 0))
-      return f;
-      
+    (!hash || memcmp(hash, f->hash, HASH_SIZE) == 0))
+   {
+       /* sent from random port */
+       if (family == AF_INET && f->rfd4 && f->rfd4->fd == fd)
+         return f;
+
+       if (family == AF_INET6 && f->rfd6 && f->rfd6->fd == fd)
+         return f;
+
+       /* sent to upstream from bound socket. */
+       if (f->sentto->sfd && f->sentto->sfd->fd == fd)
+         return f;
+   }
+
   return NULL;
 }
 
@@ -2263,15 +2273,19 @@ void server_gone(struct server *server)
 static unsigned short get_id(void)
 {
   unsigned short ret = 0;
-  
-  do 
-    ret = rand16();
-  while (lookup_frec(ret, NULL));
-  
-  return ret;
-}
-
+  struct frec *f;
 
+  while (1)
+    {
+      ret = rand16();
 
+      /* ensure id is unique. */
+      for (f = daemon->frec_list; f; f = f->next)
+       if (f->sentto && f->new_id == ret)
+         break;
 
+      if (!f)
+       return ret;
+    }
+}