From: Miod Vallat Date: Wed, 26 Feb 2025 13:31:50 +0000 (+0100) Subject: Damage control in Lua createForward() and createForward6(). X-Git-Tag: dnsdist-2.0.0-alpha1~57^2 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=97800540ef65f19cd9cb6e2624682c0840a6eafc;p=thirdparty%2Fpdns.git Damage control in Lua createForward() and createForward6(). - make sure all computed results are passed to a ComboAddress constructor, which will reject ill-formed data. This wasn't the case in createForward, when interpreting part of the requested name as an ipv4 address encoded in hexadecimal (e.g. 7f000001), but the actual name wasn't. This would otherwise end up with a SERVFAIL answer and a Lua stack traceback containing messages such as: Unable to convert presentation address '4294967292.xx.yy.zz' for a name ending with "-4" and six hex digits. - wrap these functions into a try/catch block in order to cope with possible exceptions raised by ComboAddress. This wasn't the case in createForward6 when the requested name contains at least 8 dots - this doesn't imply each component is a valid ipv6 chunk. --- diff --git a/pdns/lua-record.cc b/pdns/lua-record.cc index 886d755a63..ca0aa874ca 100644 --- a/pdns/lua-record.cc +++ b/pdns/lua-record.cc @@ -988,93 +988,96 @@ static void setupLuaRecords(LuaContext& lua) // NOLINT(readability-function-cogn }); lua.writeFunction("createForward", []() { static string allZerosIP{"0.0.0.0"}; - DNSName record_name{s_lua_record_ctx->zone_record.dr.d_name}; - if (!record_name.isWildcard()) { - return allZerosIP; - } - record_name.chopOff(); - DNSName rel{s_lua_record_ctx->qname.makeRelative(record_name)}; - - // parts is something like ["1", "2", "3", "4", "static"] or - // ["1", "2", "3", "4"] or ["ip40414243", "ip-addresses", ...] - auto parts = rel.getRawLabels(); - // Yes, this still breaks if an 1-2-3-4.XXXX is nested too deeply... - if(parts.size()>=4) { - try { - ComboAddress ca(parts[0]+"."+parts[1]+"."+parts[2]+"."+parts[3]); - return ca.toString(); - } catch (const PDNSException &e) { + try { + DNSName record_name{s_lua_record_ctx->zone_record.dr.d_name}; + if (!record_name.isWildcard()) { return allZerosIP; } - } else if (!parts.empty()) { - auto& input = parts.at(0); - - // allow a word without - in front, as long as it does not contain anything that could be a number - size_t nonhexprefix = strcspn(input.c_str(), "0123456789abcdefABCDEF"); - if (nonhexprefix > 0) { - input = input.substr(nonhexprefix); + record_name.chopOff(); + DNSName rel{s_lua_record_ctx->qname.makeRelative(record_name)}; + + // parts is something like ["1", "2", "3", "4", "static"] or + // ["1", "2", "3", "4"] or ["ip40414243", "ip-addresses", ...] + auto parts = rel.getRawLabels(); + // Yes, this still breaks if an 1-2-3-4.XXXX is nested too deeply... + if (parts.size() >= 4) { + ComboAddress address(parts[0]+"."+parts[1]+"."+parts[2]+"."+parts[3]); + return address.toString(); } + if (!parts.empty()) { + auto& input = parts.at(0); + + // allow a word without - in front, as long as it does not contain anything that could be a number + size_t nonhexprefix = strcspn(input.c_str(), "0123456789abcdefABCDEF"); + if (nonhexprefix > 0) { + input = input.substr(nonhexprefix); + } - // either hex string, or 12-13-14-15 - vector ip_parts; - - stringtok(ip_parts, input, "-"); - unsigned int x1, x2, x3, x4; - if (ip_parts.size() >= 4) { - // 1-2-3-4 with any prefix (e.g. ip-foo-bar-1-2-3-4) - string ret; - for (size_t index=4; index > 0; index--) { - auto octet = ip_parts[ip_parts.size() - index]; - try { - auto octetVal = std::stol(octet); + // either hex string, or 12-13-14-15 + vector ip_parts; + + stringtok(ip_parts, input, "-"); + if (ip_parts.size() >= 4) { + // 1-2-3-4 with any prefix (e.g. ip-foo-bar-1-2-3-4) + string ret; + for (size_t index=4; index > 0; index--) { + auto octet = ip_parts.at(ip_parts.size() - index); + auto octetVal = std::stol(octet); // may throw if (octetVal >= 0 && octetVal <= 255) { - ret += ip_parts.at(ip_parts.size() - index) + "."; + ret += octet + "."; } else { return allZerosIP; } - } catch (const std::exception &e) { - return allZerosIP; } + ret.resize(ret.size() - 1); // remove trailing dot after last octet + return ret; } - ret.resize(ret.size() - 1); // remove trailing dot after last octet - return ret; - } - if(input.length() >= 8) { - auto last8 = input.substr(input.length()-8); - if(sscanf(last8.c_str(), "%02x%02x%02x%02x", &x1, &x2, &x3, &x4)==4) { - return std::to_string(x1) + "." + std::to_string(x2) + "." + std::to_string(x3) + "." + std::to_string(x4); + if (input.length() >= 8) { + auto last8 = input.substr(input.length()-8); + unsigned int part1{0}; + unsigned int part2{0}; + unsigned int part3{0}; + unsigned int part4{0}; + if (sscanf(last8.c_str(), "%02x%02x%02x%02x", &part1, &part2, &part3, &part4) == 4) { + ComboAddress address(std::to_string(part1) + "." + std::to_string(part2) + "." + std::to_string(part3) + "." + std::to_string(part4)); + return address.toString(); + } } } + return allZerosIP; + } catch (const PDNSException &e) { + return allZerosIP; } - return allZerosIP; }); lua.writeFunction("createForward6", []() { static string allZerosIP{"::"}; - DNSName record_name{s_lua_record_ctx->zone_record.dr.d_name}; - if (!record_name.isWildcard()) { - return allZerosIP; - } - record_name.chopOff(); - DNSName rel{s_lua_record_ctx->qname.makeRelative(record_name)}; - - auto parts = rel.getRawLabels(); - if(parts.size()==8) { - string tot; - for(int i=0; i<8; ++i) { - if(i) - tot.append(1,':'); - tot+=parts[i]; + try { + DNSName record_name{s_lua_record_ctx->zone_record.dr.d_name}; + if (!record_name.isWildcard()) { + return allZerosIP; } - ComboAddress ca(tot); - return ca.toString(); - } - else if(parts.size()==1) { - if (parts[0].find('-') != std::string::npos) { - std::replace(parts[0].begin(), parts[0].end(), '-', ':'); - ComboAddress ca(parts[0]); - return ca.toString(); - } else { + record_name.chopOff(); + DNSName rel{s_lua_record_ctx->qname.makeRelative(record_name)}; + + auto parts = rel.getRawLabels(); + if (parts.size() == 8) { + string tot; + for (int chunk = 0; chunk < 8; ++chunk) { + if (chunk != 0) { + tot.append(1, ':'); + } + tot += parts.at(chunk); + } + ComboAddress address(tot); + return address.toString(); + } + if (parts.size() == 1) { + if (parts[0].find('-') != std::string::npos) { + std::replace(parts[0].begin(), parts[0].end(), '-', ':'); + ComboAddress address(parts[0]); + return address.toString(); + } if (parts[0].size() >= 32) { auto ippart = parts[0].substr(parts[0].size()-32); auto fulladdress = @@ -1087,45 +1090,49 @@ static void setupLuaRecords(LuaContext& lua) // NOLINT(readability-function-cogn ippart.substr(24, 4) + ":" + ippart.substr(28, 4); - ComboAddress ca(fulladdress); - return ca.toString(); + ComboAddress address(fulladdress); + return address.toString(); } } + return allZerosIP; + } catch (const PDNSException &e) { + return allZerosIP; } - - return allZerosIP; }); - lua.writeFunction("createReverse6", [](string format, boost::optional> e){ + lua.writeFunction("createReverse6", [](const string &format, boost::optional> excp){ vector candidates; try { auto labels= s_lua_record_ctx->qname.getRawLabels(); - if(labels.size()<32) + if (labels.size()<32) { return std::string("unknown"); + } boost::format fmt(format); fmt.exceptions( boost::io::all_error_bits ^ ( boost::io::too_many_args_bit | boost::io::too_few_args_bit ) ); string together; vector quads; - for(int i=0; i<8; ++i) { - if(i) - together+=":"; + for (int chunk = 0; chunk < 8; ++chunk) { + if (chunk != 0) { + together += ":"; + } string lquad; - for(int j=0; j <4; ++j) { - lquad.append(1, labels[31-i*4-j][0]); - together += labels[31-i*4-j][0]; + for (int quartet = 0; quartet < 4; ++quartet) { + lquad.append(1, labels[31 - chunk * 4 - quartet][0]); + together += labels[31 - chunk * 4 - quartet][0]; } quads.push_back(lquad); } - ComboAddress ip6(together,0); + ComboAddress ip6(together,0); - if(e) { - auto& addrs=*e; + if (excp) { + auto& addrs=*excp; for(const auto& addr: addrs) { // this makes sure we catch all forms of the address - if(ComboAddress(addr.first,0)==ip6) + if (ComboAddress(addr.first, 0) == ip6) { return addr.second; + } } } @@ -1142,12 +1149,14 @@ static void setupLuaRecords(LuaContext& lua) // NOLINT(readability-function-cogn dashed.insert(0, "0"); } - for(int i=31; i>=0; --i) - fmt % labels[i]; + for (int byte = 31; byte >= 0; --byte) { + fmt % labels[byte]; + } fmt % dashed; - for(const auto& lquad : quads) + for(const auto& lquad : quads) { fmt % lquad; + } return fmt.str(); } diff --git a/regression-tests.auth-py/test_LuaRecords.py b/regression-tests.auth-py/test_LuaRecords.py index 3448f22eef..23c06bcf9a 100644 --- a/regression-tests.auth-py/test_LuaRecords.py +++ b/regression-tests.auth-py/test_LuaRecords.py @@ -1046,6 +1046,8 @@ class TestLuaRecords(BaseLuaTest): "ip40414243": "64.65.66.67", "ipp40414243": "64.65.66.67", "ip4041424": "0.0.0.0", + "ip-441424": "0.0.0.0", + "ip-5abcdef": "0.0.0.0", "host64-22-33-44": "64.22.33.44", "2.2.2.2": "0.0.0.0" # filtered }), @@ -1057,7 +1059,8 @@ class TestLuaRecords(BaseLuaTest): "2001--db8" : "2001::db8", "20010002000300040005000600070db8" : "2001:2:3:4:5:6:7:db8", "blabla20010002000300040005000600070db8" : "2001:2:3:4:5:6:7:db8", - "4000-db8--1" : "fe80::1" # filtered, with fallback address override + "4000-db8--1" : "fe80::1", # filtered, with fallback address override + "l1.l2.l3.l4.l5.l6.l7.l8" : "fe80::1" }), ".createreverse6.example.org." : (dns.rdatatype.PTR, { "8.b.d.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.1.0.0.2" : "2001--db8.example.com.",