Subject: [PATCH] Security: Validate response address, possibly related to CVE-2008-1447

[PATCH] Security: Validate response address, possibly related to CVE-2008-1447

From: Brad House <brad_at_mainstreetsoftworks.com>
Date: Tue, 15 Jul 2008 12:06:15 -0400

This patch addresses an issue in which a response could be sent
back to the source port of a client from a different address than
the request was made to. This is one form of a DNS cache
poisoning attack.

The patch simply uses recvfrom() rather than recv() and
validates that the address returned from recvfrom() matches
the address of the server we have connected to. This is only
necessary on UDP sockets as they are connection-less, TCP
is unaffected.

I would consider the severity of this low as it is still
possible for an attacker to 'spoof' their address in a
crafted response message (using raw sockets).

Please reply with any comments.

-Brad

Index: ares_process.c
===================================================================
--- ares_process.c (revision 13926)
+++ ares_process.c (working copy)
@@ -380,6 +380,8 @@
   int i;
   ssize_t count;
   unsigned char buf[PACKETSZ + 1];
+ struct sockaddr_in from;
+ socklen_t fromlen;
 
   if(!read_fds && (read_fd == ARES_SOCKET_BAD))
     /* no possible action */
@@ -413,11 +415,19 @@
       /* To reduce event loop overhead, read and process as many
        * packets as we can. */
       do {
- count = sread(server->udp_socket, buf, sizeof(buf));
+ memset(&from, 0, sizeof(from));
+ fromlen = sizeof(from);
+ count = sreadfrom(server->udp_socket, buf, sizeof(buf), (struct sockaddr *) &from, &fromlen);
         if (count == -1 && try_again(SOCKERRNO))
           continue;
         else if (count <= 0)
           handle_error(channel, i, now);
+ else if (fromlen && from.sin_addr.s_addr != 0 &&
+ from.sin_addr.s_addr != server->addr.s_addr)
+ /* Address response came from did not match the address
+ * we sent the request to. Someone may be attempting
+ * to perform a cache poisoning attack */
+ break;
         else
           process_answer(channel, buf, (int)count, i, 0, now);
        } while (count > 0);
Index: setup_once.h
===================================================================
--- setup_once.h (revision 13926)
+++ setup_once.h (working copy)
@@ -115,6 +115,9 @@
                                    (RECV_TYPE_ARG2)(y), \
                                    (RECV_TYPE_ARG3)(z))
 
+#define sreadfrom(x,y,z,f,l) ((ssize_t)read((RECV_TYPE_ARG1)(x), \
+ (RECV_TYPE_ARG2)(y), \
+ (RECV_TYPE_ARG3)(z))
 #elif defined(HAVE_RECV)
 /*
  * The definitions for the return type and arguments types
@@ -151,6 +154,12 @@
                                    (RECV_TYPE_ARG2)(y), \
                                    (RECV_TYPE_ARG3)(z), \
                                    (RECV_TYPE_ARG4)(0))
+#define sreadfrom(x,y,z,f,l) (ssize_t)recvfrom((RECV_TYPE_ARG1)(x), \
+ (RECV_TYPE_ARG2)(y), \
+ (RECV_TYPE_ARG3)(z), \
+ (RECV_TYPE_ARG4)(0), \
+ (f), \
+ (l))
 #endif
 #else /* HAVE_RECV */
 #ifndef sread
Received on 2008-07-15