ares_parse_ptr_reply() handle NULL for addr/addr_len. Fixes #392

NodeJS passes NULL for addr and 0 for addrlen parameters to ares_parse_ptr_reply().  On systems where malloc(0) returned NULL, this would cause the function to return ARES_ENOMEM, but the cleanup wasn't handled properly and would crash.

This patche fixes that bug, and also hardens ares_free_hostent() to not leak memory during cleanup.

Fixes: #392
Fix By: Brad House (@bradh352)

src/lib/ares_free_hostent.c

@@ -31,11 +31,13 @@ void ares_free_hostent(struct hostent *host)
     return;
 
   ares_free((char *)(host->h_name));
-  for (p = host->h_aliases; *p; p++)
+  for (p = host->h_aliases; p && *p; p++)
     ares_free(*p);
   ares_free(host->h_aliases);
-  ares_free(host->h_addr_list[0]); /* no matter if there is one or many entries,
-                                 there is only one malloc for all of them */
-  ares_free(host->h_addr_list);
+  if (host->h_addr_list) {
+    ares_free(host->h_addr_list[0]); /* no matter if there is one or many entries,
+                                   there is only one malloc for all of them */
+    ares_free(host->h_addr_list);
+  }
   ares_free(host);
 }

src/lib/ares_parse_ptr_reply.c

@@ -48,7 +48,7 @@ int ares_parse_ptr_reply(const unsigned char *abuf, int alen, const void *addr,
   long len;
   const unsigned char *aptr;
   char *ptrname, *hostname, *rr_name, *rr_data;
-  struct hostent *hostent;
+  struct hostent *hostent = NULL;
   int aliascnt = 0;
   int alias_alloc = 8;
   char ** aliases;
@@ -175,41 +175,54 @@ int ares_parse_ptr_reply(const unsigned char *abuf, int alen, const void *addr,
     status = ARES_ENODATA;
   if (status == ARES_SUCCESS)
     {
-      /* We got our answer.  Allocate memory to build the host entry. */
-      hostent = ares_malloc(sizeof(struct hostent));
-      if (hostent)
-        {
-          hostent->h_addr_list = ares_malloc(2 * sizeof(char *));
-          if (hostent->h_addr_list)
-            {
-              hostent->h_addr_list[0] = ares_malloc(addrlen);
-              if (hostent->h_addr_list[0])
-                {
-                  hostent->h_aliases = ares_malloc((aliascnt+1) * sizeof (char *));
-                  if (hostent->h_aliases)
-                    {
-                      /* Fill in the hostent and return successfully. */
-                      hostent->h_name = hostname;
-                      for (i=0 ; i<aliascnt ; i++)
-                        hostent->h_aliases[i] = aliases[i];
-                      hostent->h_aliases[aliascnt] = NULL;
-                      hostent->h_addrtype = aresx_sitoss(family);
-                      hostent->h_length = aresx_sitoss(addrlen);
-                      memcpy(hostent->h_addr_list[0], addr, addrlen);
-                      hostent->h_addr_list[1] = NULL;
-                      *host = hostent;
-                      ares_free(aliases);
-                      ares_free(ptrname);
-                      return ARES_SUCCESS;
-                    }
-                  ares_free(hostent->h_addr_list[0]);
-                }
-              ares_free(hostent->h_addr_list);
-            }
-          ares_free(hostent);
-        }
+      /* If we don't reach the end, we must have failed due to out of memory */
       status = ARES_ENOMEM;
+
+      /* We got our answer.  Allocate memory to build the host entry. */
+      hostent = ares_malloc(sizeof(*hostent));
+      if (!hostent)
+        goto fail;
+
+      /* If we don't memset here, cleanups may fail */
+      memset(hostent, 0, sizeof(*hostent));
+
+      hostent->h_addr_list = ares_malloc(2 * sizeof(char *));
+      if (!hostent->h_addr_list)
+        goto fail;
+
+
+      if (addr && addrlen) {
+        hostent->h_addr_list[0] = ares_malloc(addrlen);
+        if (!hostent->h_addr_list[0])
+          goto fail;
+      } else {
+        hostent->h_addr_list[0] = NULL;
+      }
+
+      hostent->h_aliases = ares_malloc((aliascnt+1) * sizeof (char *));
+      if (!hostent->h_aliases)
+        goto fail;
+
+      /* Fill in the hostent and return successfully. */
+      hostent->h_name = hostname;
+      for (i=0 ; i<aliascnt ; i++)
+        hostent->h_aliases[i] = aliases[i];
+      hostent->h_aliases[aliascnt] = NULL;
+      hostent->h_addrtype = aresx_sitoss(family);
+      hostent->h_length = aresx_sitoss(addrlen);
+      if (addr && addrlen)
+        memcpy(hostent->h_addr_list[0], addr, addrlen);
+      hostent->h_addr_list[1] = NULL;
+      *host = hostent;
+      ares_free(aliases);
+      ares_free(ptrname);
+
+      return ARES_SUCCESS;
     }
+
+fail:
+  ares_free_hostent(hostent);
+
   for (i=0 ; i<aliascnt ; i++)
     if (aliases[i])
       ares_free(aliases[i]);