fix reference to freed memory (#562)

Issue #561 shows free'd memory could be accessed in some error conditions.

Fixes Issue #561
Fix By: Brad House (@bradh352)

src/lib/ares_process.c

@@ -444,7 +444,7 @@ static void read_udp_packets_fd(ares_channel channel,
         continue;
 
       handle_error(conn, now);
-
+      return;
 #ifdef HAVE_RECVFROM
     } else if (!same_address(&from.sa, &conn->server->addr)) {
       /* The address the response comes from does not match the address we
@@ -682,9 +682,10 @@ static void process_answer(ares_channel channel, const unsigned char *abuf,
 static void handle_error(struct server_connection *conn,
                          struct timeval *now)
 {
-  ares_channel        channel = conn->server->channel;
-  ares__llist_t      *list_copy;
-  ares__llist_node_t *node;
+  ares_channel         channel = conn->server->channel;
+  struct server_state *server = conn->server;
+  ares__llist_t       *list_copy;
+  ares__llist_node_t  *node;
 
   /* We steal the list from the connection then close the connection, then
    * iterate across the list to requeue any inflight queries with the broken
@@ -697,8 +698,8 @@ static void handle_error(struct server_connection *conn,
   while ((node = ares__llist_node_first(list_copy)) != NULL) {
     struct query       *query = ares__llist_node_val(node);
 
-    assert(query->server == (int)conn->server->idx);
-    skip_server(channel, query, conn->server);
+    assert(query->server == (int)server->idx);
+    skip_server(channel, query, server);
     /* next_server will remove the current node from the list */
     next_server(channel, query, now);
   }

test/ares-test-mock.cc

@@ -1041,6 +1041,26 @@ TEST_P(MockUDPChannelTest, CancelLater) {
   EXPECT_EQ(0, result.timeouts_);
 }
 
+TEST_P(MockChannelTest, DisconnectFirstAttempt) {
+  DNSPacket reply;
+  reply.set_response().set_aa()
+    .add_question(new DNSQuestion("www.google.com", T_A))
+    .add_answer(new DNSARR("www.google.com", 0x0100, {0x01, 0x02, 0x03, 0x04}));
+
+  // On second request, cancel the channel.
+  EXPECT_CALL(server_, OnRequest("www.google.com", T_A))
+    .WillOnce(Disconnect(&server_))
+    .WillOnce(SetReply(&server_, &reply));
+
+  HostResult result;
+  ares_gethostbyname(channel_, "www.google.com.", AF_INET, HostCallback, &result);
+  Process();
+  EXPECT_TRUE(result.done_);
+  std::stringstream ss;
+  ss << result.host_;
+  EXPECT_EQ("{'www.google.com' aliases=[] addrs=[1.2.3.4]}", ss.str());
+}
+
 TEST_P(MockChannelTest, GetHostByNameDestroyAbsolute) {
   HostResult result;
   ares_gethostbyname(channel_, "www.google.com.", AF_INET, HostCallback, &result);

test/ares-test.cc

@@ -390,9 +390,11 @@ void MockServer::ProcessFD(int fd) {
 
       ProcessPacket(fd, &addr, addrlen, tcp_data_ + 2, tcplen);
 
-      /* strip off processed data */
-      memmove(tcp_data_, tcp_data_ + tcplen + 2, tcp_data_len_ - 2 - tcplen);
-      tcp_data_len_ -= 2 + tcplen;
+      /* strip off processed data if connection not terminated */
+      if (tcp_data_ != NULL) {
+        memmove(tcp_data_, tcp_data_ + tcplen + 2, tcp_data_len_ - 2 - tcplen);
+        tcp_data_len_ -= 2 + tcplen;
+      }
     }
   } else {
     /* UDP is always a single packet */

test/ares-test.h

@@ -165,6 +165,15 @@ class MockServer {
   void SetReplyData(const std::vector<byte>& reply) { reply_ = reply; }
   void SetReply(const DNSPacket* reply) { SetReplyData(reply->data()); }
   void SetReplyQID(int qid) { qid_ = qid; }
+  void Disconnect() {
+    for (int fd : connfds_) {
+      sclose(fd);
+    }
+    connfds_.clear();
+    free(tcp_data_);
+    tcp_data_ = NULL;
+    tcp_data_len_ = 0;
+  }
 
   // The set of file descriptors that the server handles.
   std::set<int> fds() const;
@@ -252,6 +261,10 @@ ACTION_P2(SetReplyQID, mockserver, qid) {
 ACTION_P2(CancelChannel, mockserver, channel) {
   ares_cancel(channel);
 }
+// gMock action to disconnect all connections.
+ACTION_P2(Disconnect, mockserver) {
+  mockserver->Disconnect();
+}
 
 // C++ wrapper for struct hostent.
 struct HostEnt {