From 9da7b2be1e36e7a41383d638b2255491e8a0c98c Mon Sep 17 00:00:00 2001 From: Remi Gacogne Date: Wed, 20 Oct 2021 11:12:20 +0200 Subject: [PATCH] ZoneParserTNG: Stricted parsing of $GENERATE parameters --- pdns/test-zoneparser_tng_cc.cc | 50 ++++++++++++++++++- pdns/zoneparser-tng.cc | 91 ++++++++++++++++++++++++++++------ pdns/zoneparser-tng.hh | 1 + 3 files changed, 127 insertions(+), 15 deletions(-) diff --git a/pdns/test-zoneparser_tng_cc.cc b/pdns/test-zoneparser_tng_cc.cc index 6657109443..d435b503a8 100644 --- a/pdns/test-zoneparser_tng_cc.cc +++ b/pdns/test-zoneparser_tng_cc.cc @@ -46,7 +46,7 @@ BOOST_AUTO_TEST_CASE(test_tng_record_types) { BOOST_CHECK_EQUAL(rr.qtype.toString(), type); if (rr.qtype == QType::SOA) continue; // FIXME400 remove trailing dots from data - if (*(rr.content.rbegin()) != '.' && *(data.rbegin()) == '.') + if (*(rr.content.rbegin()) != '.' && *(data.rbegin()) == '.') BOOST_CHECK_EQUAL(rr.content, std::string(data.begin(),data.end()-1)); else BOOST_CHECK_EQUAL(rr.content, data); @@ -116,6 +116,33 @@ BOOST_AUTO_TEST_CASE(test_tng_record_generate) { BOOST_CHECK_EQUAL(rr.qtype.toString(), "A"); BOOST_CHECK_EQUAL(rr.content, "1.2.3.4"); } + { + DNSResourceRecord rr; + BOOST_CHECK(!zp.get(rr)); + } + } + + { + /* GENERATE with a larger initial counter and a large stop */ + ZoneParserTNG zp(std::vector({"$GENERATE 4294967294-4294967295/2 $ 86400 IN A 1.2.3.4"}), DNSName("unit2.test")); + + const vector expected = { + "4294967294.unit2.test.", + }; + + for (auto const & exp : expected) { + DNSResourceRecord rr; + zp.get(rr); + BOOST_CHECK_EQUAL(rr.qname.toString(), exp); + BOOST_CHECK_EQUAL(rr.ttl, 86400U); + BOOST_CHECK_EQUAL(rr.qclass, 1U); + BOOST_CHECK_EQUAL(rr.qtype.toString(), "A"); + BOOST_CHECK_EQUAL(rr.content, "1.2.3.4"); + } + { + DNSResourceRecord rr; + BOOST_CHECK(!zp.get(rr)); + } } { @@ -139,6 +166,27 @@ BOOST_AUTO_TEST_CASE(test_tng_record_generate) { BOOST_CHECK_THROW(zp.get(rr), std::exception); } + { + /* test invalid generate parameters: negative counter */ + ZoneParserTNG zp(std::vector({"$GENERATE -1-4/1 $.${1,2,o}.${3,4,d}.${5,6,X}.${7,8,x} 86400 IN A 1.2.3.4"}), DNSName("test")); + DNSResourceRecord rr; + BOOST_CHECK_THROW(zp.get(rr), std::exception); + } + + { + /* test invalid generate parameters: negative stop */ + ZoneParserTNG zp(std::vector({"$GENERATE 0--4/1 $.${1,2,o}.${3,4,d}.${5,6,X}.${7,8,x} 86400 IN A 1.2.3.4"}), DNSName("test")); + DNSResourceRecord rr; + BOOST_CHECK_THROW(zp.get(rr), std::exception); + } + + { + /* test invalid generate parameters: negative step */ + ZoneParserTNG zp(std::vector({"$GENERATE 0-4/-1 $.${1,2,o}.${3,4,d}.${5,6,X}.${7,8,x} 86400 IN A 1.2.3.4"}), DNSName("test")); + DNSResourceRecord rr; + BOOST_CHECK_THROW(zp.get(rr), std::exception); + } + { /* test invalid generate parameters: no offset */ ZoneParserTNG zp(std::vector({"$GENERATE 0-4/1 $.${}.${3,4,d}.${5,6,X}.${7,8,x} 86400 IN A 1.2.3.4"}), DNSName("test")); diff --git a/pdns/zoneparser-tng.cc b/pdns/zoneparser-tng.cc index ae2e647319..701bbbf91a 100644 --- a/pdns/zoneparser-tng.cc +++ b/pdns/zoneparser-tng.cc @@ -139,8 +139,10 @@ unsigned int ZoneParserTNG::makeTTLFromZone(const string& str) bool ZoneParserTNG::getTemplateLine() { - if(d_templateparts.empty() || d_templatecounter > d_templatestop) // no template, or done with + if (d_templateparts.empty() || d_templateCounterWrapped || d_templatecounter > d_templatestop) { + // no template, or done with return false; + } string retline; for(parts_t::const_iterator iter = d_templateparts.begin() ; iter != d_templateparts.end(); ++iter) { @@ -229,7 +231,13 @@ bool ZoneParserTNG::getTemplateLine() } retline+=outpart; } - d_templatecounter+=d_templatestep; + + if ((d_templatestop - d_templatecounter) < d_templatestep) { + d_templateCounterWrapped = true; + } + else { + d_templatecounter += d_templatestep; + } d_line = retline; return true; @@ -340,31 +348,86 @@ bool ZoneParserTNG::get(DNSResourceRecord& rr, std::string* comment) // The range part can be one of two forms: start-stop or start-stop/step. If the first // form is used, then step is set to 1. start, stop and step must be positive // integers between 0 and (2^31)-1. start must not be larger than stop. - string range=makeString(d_line, d_parts[1]); - d_templatestep=1; - d_templatestop=0; - int extracted = sscanf(range.c_str(),"%" SCNu32 "-%" SCNu32 "/%" SCNu32, &d_templatecounter, &d_templatestop, &d_templatestep); - if (extracted == 2) { - d_templatestep=1; + // http://www.zytrax.com/books/dns/ch8/generate.html + string range = makeString(d_line, d_parts.at(1)); + + auto splitOnOnlyOneSeparator = [range](const std::string& input, std::vector& output, char separator) { + output.clear(); + + auto pos = input.find(separator); + if (pos == string::npos) { + output.emplace_back(input); + return; + } + if (pos == (input.size()-1)) { + /* ends on a separator!? */ + throw std::runtime_error("Invalid range from $GENERATE parameters '" + range + "'"); + } + auto next = input.find(separator, pos + 1); + if (next != string::npos) { + /* more than one separator */ + throw std::runtime_error("Invalid range from $GENERATE parameters '" + range + "'"); + } + output.emplace_back(input.substr(0, pos)); + output.emplace_back(input.substr(pos + 1)); + }; + + std::vector fields; + splitOnOnlyOneSeparator(range, fields, '-'); + if (fields.size() != 2) { + throw std::runtime_error("Invalid range from $GENERATE parameters '" + range + "'"); } - else if (extracted != 3) { - throw exception("Invalid range from $GENERATE parameters '" + range + "'"); + + auto parseValue = [](const std::string& parameters, const std::string& name, const std::string& str, uint32_t& value) { + try { + auto got = std::stoul(str); + if (got > std::numeric_limits::max()) { + throw std::runtime_error("Invalid " + name + " value in $GENERATE parameters '" + parameters + "'"); + } + value = static_cast(got); + } + catch (const std::exception& e) { + throw std::runtime_error("Invalid " + name + " value in $GENERATE parameters '" + parameters + "': " + e.what()); + } + }; + + parseValue(range, "start", fields.at(0), d_templatecounter); + + /* now the remaining part(s) */ + range = std::move(fields.at(1)); + splitOnOnlyOneSeparator(range, fields, '/'); + + if (fields.size() > 2) { + throw std::runtime_error("Invalid range from $GENERATE parameters '" + range + "'"); } + + parseValue(range, "stop", fields.at(0), d_templatestop); + + if (fields.size() == 2) { + parseValue(range, "step", fields.at(1), d_templatestep); + } + else { + d_templatestep = 1; + } + if (d_templatestep < 1 || d_templatestop < d_templatecounter) { - throw exception("Invalid $GENERATE parameters"); + throw std::runtime_error("Invalid $GENERATE parameters"); } if (d_maxGenerateSteps != 0) { size_t numberOfSteps = (d_templatestop - d_templatecounter) / d_templatestep; if (numberOfSteps > d_maxGenerateSteps) { - throw exception("The number of $GENERATE steps (" + std::to_string(numberOfSteps) + ") is too high, the maximum is set to " + std::to_string(d_maxGenerateSteps)); + throw std::runtime_error("The number of $GENERATE steps (" + std::to_string(numberOfSteps) + ") is too high, the maximum is set to " + std::to_string(d_maxGenerateSteps)); } } - d_templateline=d_line; + + d_templateline = d_line; d_parts.pop_front(); d_parts.pop_front(); - d_templateparts=d_parts; + d_templateparts = d_parts; + d_templateCounterWrapped = false; + goto retry; } else diff --git a/pdns/zoneparser-tng.hh b/pdns/zoneparser-tng.hh index a15f7630a6..fd4464738f 100644 --- a/pdns/zoneparser-tng.hh +++ b/pdns/zoneparser-tng.hh @@ -79,4 +79,5 @@ private: bool d_fromfile; bool d_generateEnabled{true}; bool d_upgradeContent; + bool d_templateCounterWrapped{false}; }; -- 2.47.2