Skip to content

Commit 0c51d0a

Browse files
committed
dns: fix crash while using dns.setServers after dns.resolve4
The callback function in cares_query is synchronous and called before closed. So dns.setServers in the synchronous callback before closed will occur crashing. Fixes: #894 Refs: https://github.com/nodejs/node/blob/v6.9.4/deps/cares/src/ares_process.c#L1332-L1333 Refs: https://github.com/OpenSIPS/opensips/blob/2.3.0/proxy.c
1 parent f2ba06d commit 0c51d0a

File tree

2 files changed

+198
-9
lines changed

2 files changed

+198
-9
lines changed

src/cares_wrap.cc

Lines changed: 181 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ using v8::Value;
6969

7070
namespace {
7171

72+
#define ARES_HOSTENT_COPY_ERROR -1000
73+
7274
inline const char* ToErrorCodeString(int status) {
7375
switch (status) {
7476
#define V(code) case ARES_##code: return #code;
@@ -96,8 +98,10 @@ inline const char* ToErrorCodeString(int status) {
9698
V(EREFUSED)
9799
V(ESERVFAIL)
98100
V(ETIMEOUT)
101+
V(HOSTENT_COPY_ERROR)
99102
#undef V
100103
}
104+
101105
return "UNKNOWN_ARES_ERROR";
102106
}
103107

@@ -296,6 +300,121 @@ Local<Array> HostentToNames(Environment* env, struct hostent* host) {
296300
return scope.Escape(names);
297301
}
298302

303+
/* copies a hostent structure*, returns 0 on success, -1 on error */
304+
/* this function refers to OpenSIPS */
305+
int cares_wrap_hostent_cpy(struct hostent *dst, struct hostent* src) {
306+
unsigned int len, len2, i, r;
307+
308+
/* start copying the host entry.. */
309+
/* copy h_name */
310+
len = strlen(src->h_name) + 1;
311+
dst->h_name = reinterpret_cast<char*>(node::Malloc(sizeof(char) * len));
312+
if (dst->h_name) {
313+
strncpy(dst->h_name, src->h_name, len);
314+
} else {
315+
return -1;
316+
}
317+
318+
/* copy h_aliases */
319+
len = 0;
320+
if (src->h_aliases) {
321+
for (; src->h_aliases[len]; len++) {}
322+
}
323+
324+
dst->h_aliases = reinterpret_cast<char**>(
325+
node::Malloc(sizeof(char*) * (len + 1)));
326+
327+
if (dst->h_aliases == 0) {
328+
free(dst->h_name);
329+
return -1;
330+
}
331+
332+
memset(reinterpret_cast<void*>(dst->h_aliases), 0, sizeof(char*) * (len + 1));
333+
for (i = 0; i < len; i++) {
334+
len2 = strlen(src->h_aliases[i]) + 1;
335+
dst->h_aliases[i] = reinterpret_cast<char*>(
336+
node::Malloc(sizeof(char) * len2));
337+
338+
if (dst->h_aliases[i] == 0) {
339+
free(dst->h_name);
340+
for (r = 0; r < i; r++) free(dst->h_aliases[r]);
341+
free(dst->h_aliases);
342+
return -1;
343+
}
344+
345+
strncpy(dst->h_aliases[i], src->h_aliases[i], len2);
346+
}
347+
348+
/* copy h_addr_list */
349+
len = 0;
350+
if (src->h_addr_list) {
351+
for (; src->h_addr_list[len]; len++) {}
352+
}
353+
354+
dst->h_addr_list = reinterpret_cast<char**>(
355+
node::Malloc(sizeof(char*) * (len + 1)));
356+
357+
if (dst->h_addr_list == 0) {
358+
free(dst->h_name);
359+
for (r = 0; dst->h_aliases[r]; r++) free(dst->h_aliases[r]);
360+
free(dst->h_aliases);
361+
return -1;
362+
}
363+
364+
memset(reinterpret_cast<void*>(
365+
dst->h_addr_list), 0, sizeof(char*) * (len + 1));
366+
367+
for (i=0; i < len; i++) {
368+
dst->h_addr_list[i] = reinterpret_cast<char*>(
369+
node::Malloc(sizeof(char) * src->h_length));
370+
if (dst->h_addr_list[i] == 0) {
371+
free(dst->h_name);
372+
for (r = 0; dst->h_aliases[r]; r++) free(dst->h_aliases[r]);
373+
free(dst->h_aliases);
374+
for (r = 0; r < i; r++) free(dst->h_addr_list[r]);
375+
free(dst->h_addr_list);
376+
return -1;
377+
}
378+
379+
memcpy(dst->h_addr_list[i], src->h_addr_list[i], src->h_length);
380+
}
381+
382+
/* copy h_addr_type & length */
383+
dst->h_addrtype = src->h_addrtype;
384+
dst->h_length = src->h_length;
385+
/*finished hostent copy */
386+
387+
return 0;
388+
}
389+
390+
/* this function refers to OpenSIPS */
391+
void cares_wrap_free_hostent(struct hostent *dst) {
392+
int r;
393+
if (dst->h_name) free(dst->h_name);
394+
if (dst->h_aliases) {
395+
for (r = 0; dst->h_aliases[r]; r++) {
396+
free(dst->h_aliases[r]);
397+
}
398+
free(dst->h_aliases);
399+
}
400+
if (dst->h_addr_list) {
401+
for (r = 0; dst->h_addr_list[r]; r++) {
402+
free(dst->h_addr_list[r]);
403+
}
404+
free(dst->h_addr_list);
405+
}
406+
}
407+
408+
class QueryWrap;
409+
struct CaresAsyncData {
410+
QueryWrap* wrap;
411+
int status;
412+
bool is_host;
413+
void* buf;
414+
int len;
415+
416+
uv_async_t async_handle;
417+
};
299418

300419
class QueryWrap : public AsyncWrap {
301420
public:
@@ -328,30 +447,83 @@ class QueryWrap : public AsyncWrap {
328447
return static_cast<void*>(this);
329448
}
330449

331-
static void Callback(void *arg, int status, int timeouts,
332-
unsigned char* answer_buf, int answer_len) {
333-
QueryWrap* wrap = static_cast<QueryWrap*>(arg);
450+
static void CaresAsyncClose(uv_handle_t* handle) {
451+
uv_async_t* async = reinterpret_cast<uv_async_t*>(handle);
452+
struct CaresAsyncData* data =
453+
static_cast<struct CaresAsyncData*>(async->data);
454+
delete data->wrap;
455+
delete data;
456+
}
457+
458+
static void CaresAsyncCb(uv_async_t* handle) {
459+
struct CaresAsyncData* data =
460+
static_cast<struct CaresAsyncData*>(handle->data);
461+
462+
QueryWrap* wrap = data->wrap;
463+
int status = data->status;
334464

335465
if (status != ARES_SUCCESS) {
336466
wrap->ParseError(status);
467+
} else if (!data->is_host) {
468+
unsigned char* buf = reinterpret_cast<unsigned char*>(data->buf);
469+
wrap->Parse(buf, data->len);
470+
free(buf);
337471
} else {
338-
wrap->Parse(answer_buf, answer_len);
472+
hostent* host = static_cast<struct hostent*>(data->buf);
473+
wrap->Parse(host);
474+
cares_wrap_free_hostent(host);
475+
free(host);
339476
}
340477

341-
delete wrap;
478+
uv_close(reinterpret_cast<uv_handle_t*>(handle), CaresAsyncClose);
479+
}
480+
481+
static void Callback(void *arg, int status, int timeouts,
482+
unsigned char* answer_buf, int answer_len) {
483+
QueryWrap* wrap = static_cast<QueryWrap*>(arg);
484+
485+
unsigned char* buf_copy = (unsigned char*)node::Malloc(
486+
sizeof(unsigned char) * answer_len);
487+
memcpy(buf_copy, answer_buf, sizeof(unsigned char) * answer_len);
488+
489+
struct CaresAsyncData* data = new struct CaresAsyncData();
490+
data->status = status;
491+
data->wrap = wrap;
492+
data->is_host = false;
493+
data->buf = buf_copy;
494+
data->len = answer_len;
495+
496+
uv_async_t* async_handle = &data->async_handle;
497+
uv_async_init(wrap->env()->event_loop(), async_handle, CaresAsyncCb);
498+
499+
async_handle->data = data;
500+
uv_async_send(async_handle);
342501
}
343502

344503
static void Callback(void *arg, int status, int timeouts,
345504
struct hostent* host) {
346505
QueryWrap* wrap = static_cast<QueryWrap*>(arg);
347506

348-
if (status != ARES_SUCCESS) {
349-
wrap->ParseError(status);
507+
struct hostent* host_copy = (struct hostent*)node::Malloc(sizeof(hostent));
508+
int ret = cares_wrap_hostent_cpy(host_copy, host);
509+
510+
struct CaresAsyncData* data = new struct CaresAsyncData();
511+
if (ret == 0) {
512+
data->status = status;
513+
data->buf = host_copy;
350514
} else {
351-
wrap->Parse(host);
515+
data->status = ARES_HOSTENT_COPY_ERROR;
516+
data->buf = nullptr;
517+
free(host_copy);
352518
}
519+
data->wrap = wrap;
520+
data->is_host = true;
353521

354-
delete wrap;
522+
uv_async_t* async_handle = &data->async_handle;
523+
uv_async_init(wrap->env()->event_loop(), async_handle, CaresAsyncCb);
524+
525+
async_handle->data = data;
526+
uv_async_send(async_handle);
355527
}
356528

357529
void CallOnComplete(Local<Value> answer,
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
'use strict';
2+
3+
const common = require('../common');
4+
const dns = require('dns');
5+
6+
dns.resolve4('google.com', common.mustCall(function(/* err, nameServers */) {
7+
// do not care about `err` and `nameServers`,
8+
// both failed and succeeded would be OK.
9+
//
10+
// just to test crash
11+
12+
dns.setServers([ '8.8.8.8' ]);
13+
14+
// the test case shouldn't crash here
15+
// see https://github.com/nodejs/node/pull/13050 and
16+
// https://github.com/nodejs/node/issues/894
17+
}));

0 commit comments

Comments
 (0)