OSSForks/node
1
0
Fork 0
mirror of https://github.com/zebrajr/node.git synced 2026-01-15 12:15:26 +00:00
Code Issues Actions 8 Packages Projects Releases Wiki Activity

src: fix freeing unintialized pointer bug in ParseSoaReply

Browse Source 
ares_expand_name doesn't guarantee that pointer variable is initialized
if return code is ARES_EBADNAME or ARES_ENOMEM. But current usage of the
function in the codebase thinks otherwise.

There seems to be an assumption that pointer is always initialized even
though it is a local variable and we create a unique pointer soon after
calling ares_expand_name. This could potentially crash the program with
an invalid free pointer.

I was able to crash it by poisoning the memory and some manual hooks.

By moving the unique_ptr after checking the return code we can fix the
problem. As the underlying function guarantees that pointer is
initialized when the status is ARES_SUCCESS.

PR-URL: https://github.com/nodejs/node/pull/35502
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
This commit is contained in:
Aastha Gupta
2020-10-04 20:19:51 +05:30
committed by Myles Borins
parent dec004f742
commit 2d5393bb28
1 changed files with 10 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

18
src/cares_wrap.cc
View File

@@ -1067,29 +1067,31 @@ int ParseSoaReply(Environment* env,
// Can't use ares_parse_soa_reply() here which can only parse single record
const unsigned int ancount = cares_get_16bit(buf + 6);
unsigned char* ptr = buf + NS_HFIXEDSZ;
char* name_temp;
char* name_temp = nullptr;
long temp_len; // NOLINT(runtime/int)
int status = ares_expand_name(ptr, buf, len, &name_temp, &temp_len);
const ares_unique_ptr name(name_temp);
if (status != ARES_SUCCESS) {
// returns EBADRESP in case of invalid input
return status == ARES_EBADNAME ? ARES_EBADRESP : status;
}
const ares_unique_ptr name(name_temp);
if (ptr + temp_len + NS_QFIXEDSZ > buf + len) {
return ARES_EBADRESP;
}
ptr += temp_len + NS_QFIXEDSZ;
for (unsigned int i = 0; i < ancount; i++) {
char* rr_name_temp;
char* rr_name_temp = nullptr;
long rr_temp_len; // NOLINT(runtime/int)
int status2 = ares_expand_name(ptr, buf, len, &rr_name_temp, &rr_temp_len);
const ares_unique_ptr rr_name(rr_name_temp);
if (status2 != ARES_SUCCESS)
return status2 == ARES_EBADNAME ? ARES_EBADRESP : status2;
const ares_unique_ptr rr_name(rr_name_temp);
ptr += rr_temp_len;
if (ptr + NS_RRFIXEDSZ > buf + len) {
return ARES_EBADRESP;
@@ -1101,27 +1103,27 @@ int ParseSoaReply(Environment* env,
// only need SOA
if (rr_type == ns_t_soa) {
char* nsname_temp;
char* nsname_temp = nullptr;
long nsname_temp_len; // NOLINT(runtime/int)
int status3 = ares_expand_name(ptr, buf, len,
&nsname_temp,
&nsname_temp_len);
const ares_unique_ptr nsname(nsname_temp);
if (status3 != ARES_SUCCESS) {
return status3 == ARES_EBADNAME ? ARES_EBADRESP : status3;
}
const ares_unique_ptr nsname(nsname_temp);
ptr += nsname_temp_len;
char* hostmaster_temp;
char* hostmaster_temp = nullptr;
long hostmaster_temp_len; // NOLINT(runtime/int)
int status4 = ares_expand_name(ptr, buf, len,
&hostmaster_temp,
&hostmaster_temp_len);
const ares_unique_ptr hostmaster(hostmaster_temp);
if (status4 != ARES_SUCCESS) {
return status4 == ARES_EBADNAME ? ARES_EBADRESP : status4;
}
const ares_unique_ptr hostmaster(hostmaster_temp);
ptr += hostmaster_temp_len;
if (ptr + 5 * 4 > buf + len) {