Skip to content

Commit d3caea7

Browse files
XadillaXMylesBorins
authored andcommitted
dns: fix crash using dns.setServers after 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 Backport-PR-URL: #13724 PR-URL: #13050 Reviewed-By: Anna Henningsen <[email protected]>
1 parent 43fcb8f commit d3caea7

File tree

2 files changed

+164
-11
lines changed

2 files changed

+164
-11
lines changed

src/cares_wrap.cc

+152-11
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ inline const char* ToErrorCodeString(int status) {
7676
V(ETIMEOUT)
7777
#undef V
7878
}
79+
7980
return "UNKNOWN_ARES_ERROR";
8081
}
8182

@@ -280,6 +281,96 @@ static Local<Array> HostentToNames(Environment* env, struct hostent* host) {
280281
return scope.Escape(names);
281282
}
282283

284+
void safe_free_hostent(struct hostent* host) {
285+
int idx;
286+
287+
if (host->h_addr_list != nullptr) {
288+
idx = 0;
289+
while (host->h_addr_list[idx]) {
290+
free(host->h_addr_list[idx++]);
291+
}
292+
free(host->h_addr_list);
293+
host->h_addr_list = 0;
294+
}
295+
296+
if (host->h_aliases != nullptr) {
297+
idx = 0;
298+
while (host->h_aliases[idx]) {
299+
free(host->h_aliases[idx++]);
300+
}
301+
free(host->h_aliases);
302+
host->h_aliases = 0;
303+
}
304+
305+
if (host->h_name != nullptr) {
306+
free(host->h_name);
307+
}
308+
309+
host->h_addrtype = host->h_length = 0;
310+
}
311+
312+
void cares_wrap_hostent_cpy(struct hostent* dest, struct hostent* src) {
313+
dest->h_addr_list = nullptr;
314+
dest->h_addrtype = 0;
315+
dest->h_aliases = nullptr;
316+
dest->h_length = 0;
317+
dest->h_name = nullptr;
318+
319+
/* copy `h_name` */
320+
size_t name_size = strlen(src->h_name) + 1;
321+
dest->h_name = static_cast<char*>(node::Malloc(name_size));
322+
memcpy(dest->h_name, src->h_name, name_size);
323+
324+
/* copy `h_aliases` */
325+
size_t alias_count;
326+
size_t cur_alias_length;
327+
for (alias_count = 0;
328+
src->h_aliases[alias_count] != nullptr;
329+
alias_count++) {
330+
}
331+
332+
dest->h_aliases = static_cast<char**>(node::Malloc((alias_count + 1) *
333+
sizeof(char*)));
334+
for (size_t i = 0; i < alias_count; i++) {
335+
cur_alias_length = strlen(src->h_aliases[i]);
336+
dest->h_aliases[i] = static_cast<char*>(node::Malloc(cur_alias_length + 1));
337+
memcpy(dest->h_aliases[i], src->h_aliases[i], cur_alias_length + 1);
338+
}
339+
dest->h_aliases[alias_count] = nullptr;
340+
341+
/* copy `h_addr_list` */
342+
size_t list_count;
343+
for (list_count = 0;
344+
src->h_addr_list[list_count] != nullptr;
345+
list_count++) {
346+
}
347+
348+
dest->h_addr_list = static_cast<char**>(node::Malloc((list_count + 1) *
349+
sizeof(char*)));
350+
for (size_t i = 0; i < list_count; i++) {
351+
dest->h_addr_list[i] = static_cast<char*>(node::Malloc(src->h_length));
352+
memcpy(dest->h_addr_list[i], src->h_addr_list[i], src->h_length);
353+
}
354+
dest->h_addr_list[list_count] = nullptr;
355+
356+
/* work after work */
357+
dest->h_length = src->h_length;
358+
dest->h_addrtype = src->h_addrtype;
359+
}
360+
361+
class QueryWrap;
362+
struct CaresAsyncData {
363+
QueryWrap* wrap;
364+
int status;
365+
bool is_host;
366+
union {
367+
hostent* host;
368+
unsigned char* buf;
369+
} data;
370+
int len;
371+
372+
uv_async_t async_handle;
373+
};
283374

284375
class QueryWrap : public AsyncWrap {
285376
public:
@@ -311,30 +402,80 @@ class QueryWrap : public AsyncWrap {
311402
return static_cast<void*>(this);
312403
}
313404

314-
static void Callback(void *arg, int status, int timeouts,
315-
unsigned char* answer_buf, int answer_len) {
316-
QueryWrap* wrap = static_cast<QueryWrap*>(arg);
405+
static void CaresAsyncClose(uv_handle_t* handle) {
406+
uv_async_t* async = reinterpret_cast<uv_async_t*>(handle);
407+
auto data = static_cast<struct CaresAsyncData*>(async->data);
408+
delete data->wrap;
409+
delete data;
410+
}
411+
412+
static void CaresAsyncCb(uv_async_t* handle) {
413+
auto data = static_cast<struct CaresAsyncData*>(handle->data);
414+
415+
QueryWrap* wrap = data->wrap;
416+
int status = data->status;
317417

318418
if (status != ARES_SUCCESS) {
319419
wrap->ParseError(status);
420+
} else if (!data->is_host) {
421+
unsigned char* buf = data->data.buf;
422+
wrap->Parse(buf, data->len);
423+
free(buf);
320424
} else {
321-
wrap->Parse(answer_buf, answer_len);
425+
hostent* host = data->data.host;
426+
wrap->Parse(host);
427+
safe_free_hostent(host);
428+
free(host);
322429
}
323430

324-
delete wrap;
431+
uv_close(reinterpret_cast<uv_handle_t*>(handle), CaresAsyncClose);
325432
}
326433

327434
static void Callback(void *arg, int status, int timeouts,
328-
struct hostent* host) {
435+
unsigned char* answer_buf, int answer_len) {
329436
QueryWrap* wrap = static_cast<QueryWrap*>(arg);
330437

331-
if (status != ARES_SUCCESS) {
332-
wrap->ParseError(status);
333-
} else {
334-
wrap->Parse(host);
438+
unsigned char* buf_copy = nullptr;
439+
if (status == ARES_SUCCESS) {
440+
buf_copy = static_cast<unsigned char*>(node::Malloc(answer_len));
441+
memcpy(buf_copy, answer_buf, answer_len);
335442
}
336443

337-
delete wrap;
444+
CaresAsyncData* data = new CaresAsyncData();
445+
data->status = status;
446+
data->wrap = wrap;
447+
data->is_host = false;
448+
data->data.buf = buf_copy;
449+
data->len = answer_len;
450+
451+
uv_async_t* async_handle = &data->async_handle;
452+
uv_async_init(wrap->env()->event_loop(), async_handle, CaresAsyncCb);
453+
454+
async_handle->data = data;
455+
uv_async_send(async_handle);
456+
}
457+
458+
static void Callback(void *arg, int status, int timeouts,
459+
struct hostent* host) {
460+
QueryWrap* wrap = static_cast<QueryWrap*>(arg);
461+
462+
struct hostent* host_copy = nullptr;
463+
if (status == ARES_SUCCESS) {
464+
host_copy = static_cast<hostent*>(node::Malloc(sizeof(hostent)));
465+
cares_wrap_hostent_cpy(host_copy, host);
466+
}
467+
468+
CaresAsyncData* data = new CaresAsyncData();
469+
data->status = status;
470+
data->data.host = host_copy;
471+
data->wrap = wrap;
472+
data->is_host = true;
473+
474+
uv_async_t* async_handle = &data->async_handle;
475+
uv_async_init(wrap->env()->event_loop(), async_handle, CaresAsyncCb);
476+
477+
async_handle->data = data;
478+
uv_async_send(async_handle);
338479
}
339480

340481
void CallOnComplete(Local<Value> answer,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
'use strict';
2+
3+
// We don't care about `err` in the callback function of `dns.resolve4`. We just
4+
// want to test whether `dns.setServers` that is run after `resolve4` will cause
5+
// a crash or not. If it doesn't crash, the test succeeded.
6+
7+
const common = require('../common');
8+
const dns = require('dns');
9+
10+
dns.resolve4('google.com', common.mustCall(function(/* err, nameServers */) {
11+
dns.setServers([ '8.8.8.8' ]);
12+
}));

0 commit comments

Comments
 (0)