diff options
author | Aastha Gupta <aastha.gupta4104@gmail.com> | 2020-10-04 17:49:51 +0300 |
---|---|---|
committer | Rich Trott <rtrott@gmail.com> | 2020-10-08 14:38:46 +0300 |
commit | 0f41bcabf32d0979093f36b4c41959697946ba12 (patch) | |
tree | f863818b54f9ce890103ab5bfe32d0292fdfe6ca /src/cares_wrap.cc | |
parent | ee5f849fda6afcedb3a39bb07a29a771f87d7676 (diff) |
src: fix freeing unintialized pointer bug in ParseSoaReply
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>
Diffstat (limited to 'src/cares_wrap.cc')
-rw-r--r-- | src/cares_wrap.cc | 18 |
1 files changed, 10 insertions, 8 deletions
diff --git a/src/cares_wrap.cc b/src/cares_wrap.cc index defe1ef3749..cc7ad316d2b 100644 --- a/src/cares_wrap.cc +++ b/src/cares_wrap.cc @@ -1060,29 +1060,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; @@ -1094,27 +1096,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) { |