dns: fix crash while using dns.setServers after dns.resolve4

src/cares_wrap.cc

@@ -69,6 +69,8 @@ using v8::Value;
 
 namespace {
 
+#define ARES_HOSTENT_COPY_ERROR -1000
+
 inline const char* ToErrorCodeString(int status) {
   switch (status) {
 #define V(code) case ARES_##code: return #code;
@@ -96,8 +98,10 @@ inline const char* ToErrorCodeString(int status) {
     V(EREFUSED)
     V(ESERVFAIL)
     V(ETIMEOUT)
+    V(HOSTENT_COPY_ERROR)
 #undef V
   }
+
   return "UNKNOWN_ARES_ERROR";
 }
 
@@ -296,6 +300,93 @@ Local<Array> HostentToNames(Environment* env, struct hostent* host) {
   return scope.Escape(names);
 }
 
+void safe_free_hostent(struct hostent* host) {
+  int idx;
+
+  if (host->h_addr_list) {
+    idx = 0;
+    while (host->h_addr_list[idx]) {
+      free(host->h_addr_list[idx++]);
+    }
+    free(host->h_addr_list);
+    host->h_addr_list = 0;
+  }
+
+  if (host->h_aliases) {
+    idx = 0;
+    while (host->h_aliases[idx]) {
+      free(host->h_aliases[idx++]);
+    }
+    free(host->h_aliases);
+    host->h_aliases = 0;
+  }
+
+  if (host->h_name) {
+    free(host->h_name);
+  }
+
+  host->h_addrtype = host->h_length = 0;
+}
+
+bool cares_wrap_hostent_cpy(struct hostent* dest, struct hostent* src) {
+  dest->h_addr_list = nullptr;
+  dest->h_addrtype = 0;
+  dest->h_aliases = nullptr;
+  dest->h_length = 0;
+  dest->h_name = nullptr;
+
+  /* copy `h_name` */
+  size_t name_size = strlen(src->h_name) + 1;
+  dest->h_name = node::Malloc<char>(name_size);
+  memcpy(dest->h_name, src->h_name, name_size);
+
+  /* copy `h_aliases` */
+  size_t alias_count;
+  size_t cur_alias_length;
+  for (alias_count = 0;
+      src->h_aliases[alias_count] != nullptr;
+      alias_count++) {
+  }
+
+  dest->h_aliases = node::Malloc<char*>(alias_count + 1);
+  memset(dest->h_aliases, 0, sizeof(char*) * (alias_count + 1));
+  for (size_t i = 0; i < alias_count; i++) {
+    cur_alias_length = strlen(src->h_aliases[i]);
+    dest->h_aliases[i] = node::Malloc<char>(cur_alias_length + 1);
+    memcpy(dest->h_aliases[i], src->h_aliases[i], cur_alias_length + 1);
+  }
+
+  /* copy `h_addr_list` */
+  unsigned int list_count;
+  for (list_count = 0;
+      src->h_addr_list[list_count] != nullptr;
+      list_count++) {
+  }
+
+  dest->h_addr_list = node::Malloc<char*>(list_count + 1);
+  memset(dest->h_addr_list, 0, sizeof(char*) * (list_count + 1));
+  for (unsigned int i = 0; i < list_count; i++) {
+    dest->h_addr_list[i] = node::Malloc<char>(src->h_length);
+    memcpy(dest->h_addr_list[i], src->h_addr_list[i], src->h_length);
+  }
+
+  /* work after work */
+  dest->h_length = src->h_length;
+  dest->h_addrtype = src->h_addrtype;
+
+  return true;
+}
+
+class QueryWrap;
+struct CaresAsyncData {
+  QueryWrap* wrap;
+  int status;
+  bool is_host;
+  void* buf;
+  int len;
+
+  uv_async_t async_handle;
+};
 
 class QueryWrap : public AsyncWrap {
  public:
@@ -328,30 +419,88 @@ class QueryWrap : public AsyncWrap {
     return static_cast<void*>(this);
   }
 
-  static void Callback(void *arg, int status, int timeouts,
-      unsigned char* answer_buf, int answer_len) {
-    QueryWrap* wrap = static_cast<QueryWrap*>(arg);
+  static void CaresAsyncClose(uv_handle_t* handle) {
+    uv_async_t* async = reinterpret_cast<uv_async_t*>(handle);
+    auto data = static_cast<struct CaresAsyncData*>(async->data);
+    delete data->wrap;
+    delete data;
+  }
+
+  static void CaresAsyncCb(uv_async_t* handle) {
+    auto data = static_cast<struct CaresAsyncData*>(handle->data);
+
+    QueryWrap* wrap = data->wrap;
+    int status = data->status;
 
     if (status != ARES_SUCCESS) {
       wrap->ParseError(status);
+    } else if (!data->is_host) {
+      unsigned char* buf = reinterpret_cast<unsigned char*>(data->buf);
+      wrap->Parse(buf, data->len);
+      free(buf);
     } else {
-      wrap->Parse(answer_buf, answer_len);
+      hostent* host = static_cast<struct hostent*>(data->buf);
+      wrap->Parse(host);
+      safe_free_hostent(host);
+      free(host);
     }
 
-    delete wrap;
+    uv_close(reinterpret_cast<uv_handle_t*>(handle), CaresAsyncClose);
+  }
+
+  static void Callback(void *arg, int status, int timeouts,
+      unsigned char* answer_buf, int answer_len) {
+    QueryWrap* wrap = static_cast<QueryWrap*>(arg);
+
+    unsigned char* buf_copy = nullptr;
+    if (status == ARES_SUCCESS) {
+      buf_copy = node::Malloc<unsigned char>(answer_len);
+      memcpy(buf_copy, answer_buf, answer_len);
+    }
+
+    struct CaresAsyncData* data = new struct CaresAsyncData();
+    data->status = status;
+    data->wrap = wrap;
+    data->is_host = false;
+    data->buf = buf_copy;
+    data->len = answer_len;
+
+    uv_async_t* async_handle = &data->async_handle;
+    uv_async_init(wrap->env()->event_loop(), async_handle, CaresAsyncCb);
+
+    async_handle->data = data;
+    uv_async_send(async_handle);
   }
 
   static void Callback(void *arg, int status, int timeouts,
       struct hostent* host) {
     QueryWrap* wrap = static_cast<QueryWrap*>(arg);
 
-    if (status != ARES_SUCCESS) {
-      wrap->ParseError(status);
+    struct hostent* host_copy = nullptr;
+    bool copy_ret = false;
+
+    if (status == ARES_SUCCESS) {
+      host_copy = node::Malloc<hostent>(1);
+      copy_ret = cares_wrap_hostent_cpy(host_copy, host);
+    }
+
+    struct CaresAsyncData* data = new struct CaresAsyncData();
+    if (copy_ret) {
+      data->status = status;
+      data->buf = host_copy;
     } else {
-      wrap->Parse(host);
+      data->status = ARES_HOSTENT_COPY_ERROR;
+      data->buf = nullptr;
+      free(host_copy);
     }
+    data->wrap = wrap;
+    data->is_host = true;
 
-    delete wrap;
+    uv_async_t* async_handle = &data->async_handle;
+    uv_async_init(wrap->env()->event_loop(), async_handle, CaresAsyncCb);
+
+    async_handle->data = data;
+    uv_async_send(async_handle);
   }
 
   void CallOnComplete(Local<Value> answer,

test/internet/test-dns-setserver-in-callback-of-resolve4.js

@@ -0,0 +1,13 @@
+'use strict';
+
+// Refs: https://github.com/nodejs/node/pull/13050
+// We don't care about `err` in the callback function of `dns.resolve4` here.
+// We just want to test whether `dns.setServers` here in the callback of
+// `resolve4` will crash or not. If no crashing here, the test is succeeded.
+
+const common = require('../common');
+const dns = require('dns');
+
+dns.resolve4('google.com', common.mustCall(function(/* err, nameServers */) {
+  dns.setServers([ '8.8.8.8' ]);
+}));