diff --git a/CHANGELOG b/CHANGELOG index f6f4d79e5..2755059a6 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,5 +1,7 @@ #Nmap Changelog ($Id$); -*-text-*- +o [GH#3414] Avoid out-of-bounds read when parsing PTR domain names. [@toor11 (Juri)] + o Fixed an issue with parsing nmap-service-probes that could cause a corrupted file to crash Nmap. Reported by Muhammed Hussein. diff --git a/libnetutil/massdns.cc b/libnetutil/massdns.cc index b0b5bbca3..d9ba155c7 100644 --- a/libnetutil/massdns.cc +++ b/libnetutil/massdns.cc @@ -1713,8 +1713,11 @@ bool DNS::Factory::ptrToIp(const std::string &ptr, sockaddr_storage &ip) memset(&ip, 0, sizeof(sockaddr_storage)); - // Check whether the name ends with the IPv4 PTR domain - if (NULL != (p = strcasestr(cptr + ptr.length() + 1 - sizeof(C_IPV4_PTR_DOMAIN), C_IPV4_PTR_DOMAIN))) + // Check whether the name ends with the IPv4 PTR domain. The length check + // keeps the search from starting before the beginning of the name: a name + // shorter than the suffix cannot end with it. + if (ptr.length() >= sizeof(C_IPV4_PTR_DOMAIN) - 1 + && NULL != (p = strcasestr(cptr + ptr.length() + 1 - sizeof(C_IPV4_PTR_DOMAIN), C_IPV4_PTR_DOMAIN))) { struct sockaddr_in *ip4 = (struct sockaddr_in *)&ip; static const u8 place_value[] = {1, 10, 100}; @@ -1749,7 +1752,8 @@ bool DNS::Factory::ptrToIp(const std::string &ptr, sockaddr_storage &ip) ip.ss_family = AF_INET; } // If not, check IPv6 - else if (NULL != (p = strcasestr(cptr + ptr.length() + 1 - sizeof(C_IPV6_PTR_DOMAIN), C_IPV6_PTR_DOMAIN))) + else if (ptr.length() >= sizeof(C_IPV6_PTR_DOMAIN) - 1 + && NULL != (p = strcasestr(cptr + ptr.length() + 1 - sizeof(C_IPV6_PTR_DOMAIN), C_IPV6_PTR_DOMAIN))) { struct sockaddr_in6 *ip6 = (struct sockaddr_in6 *)&ip; u8 alt = 0; diff --git a/tests/nmap_dns_test.cc b/tests/nmap_dns_test.cc index 876c43984..98e3dfefd 100644 --- a/tests/nmap_dns_test.cc +++ b/tests/nmap_dns_test.cc @@ -252,6 +252,26 @@ o.debugging = 1; TEST_INCR(a->length == 0x01, ret, tot); TEST_INCR(a->ttl == 86392, ret, tot); + // Names shorter than the PTR suffixes must not be treated as PTR names. + // The root label above ("." from a server-supplied answer) is the shortest + // such name; ptrToIp must reject these without searching before the start + // of the name. + sockaddr_storage short_ip; + const char *short_names[] = { ".", "a.com", "x", "ip6.arpa" }; + for (size_t si = 0; si < sizeof(short_names) / sizeof(short_names[0]); si++) { + DNS::Factory::ptrToIp(short_names[si], short_ip); + TEST_INCR(short_ip.ss_family == AF_UNSPEC, ret, tot); + } + + // Well-formed PTR names still resolve, in either case. + sockaddr_storage case_ip; + TEST_INCR(DNS::Factory::ptrToIp("156.32.33.45.in-addr.arpa", case_ip), ret, tot); + TEST_INCR(case_ip.ss_family == AF_INET, ret, tot); + TEST_INCR(((sockaddr_in *)&case_ip)->sin_addr.s_addr == htonl(0x2d21209c), ret, tot); + TEST_INCR(DNS::Factory::ptrToIp("156.32.33.45.IN-ADDR.ARPA", case_ip), ret, tot); + TEST_INCR(case_ip.ss_family == AF_INET, ret, tot); + TEST_INCR(((sockaddr_in *)&case_ip)->sin_addr.s_addr == htonl(0x2d21209c), ret, tot); + if(ret) std::cout << "Testing nmap_dns finished with errors" << std::endl; else std::cout << "Testing nmap_dns finished without errors" << std::endl; std::cout << "Ran " << tot << " tests. " << ret << " failures." << std::endl;