From: Otto Moerbeek Date: Wed, 20 Oct 2021 09:47:36 +0000 (+0200) Subject: Limit #include depth X-Git-Tag: rec-4.6.0-beta1~19^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e3fc3ebd0f461965fd796e94b6c37271dab3ad29;p=thirdparty%2Fpdns.git Limit #include depth --- diff --git a/docs/settings.rst b/docs/settings.rst index e983d43195..308a505d95 100644 --- a/docs/settings.rst +++ b/docs/settings.rst @@ -1011,6 +1011,17 @@ will generally suffice for most installations. Maximum number of empty non-terminals to add to a zone. This is a protection measure to avoid database explosion due to long names. +.. _setting-max-include-depth: + +``max-include-depth`` +---------------------- + +- Integer +- Default: 20 + +Maximum number of nested ``$INCLUDE`` directives while processing a zone file. +Zero mean no ``$INCLUDE`` directives will be accepted. + .. _setting-max-generate-steps: ``max-generate-steps`` diff --git a/modules/bindbackend/bindbackend2.cc b/modules/bindbackend/bindbackend2.cc index 216b1b9cb3..54d9c612ba 100644 --- a/modules/bindbackend/bindbackend2.cc +++ b/modules/bindbackend/bindbackend2.cc @@ -503,6 +503,7 @@ void Bind2Backend::parseZoneFile(BB2DomainInfo* bbd) auto records = std::make_shared(); ZoneParserTNG zpt(bbd->d_filename, bbd->d_name, s_binddirectory, d_upgradeContent); zpt.setMaxGenerateSteps(::arg().asNum("max-generate-steps")); + zpt.setMaxIncludes(::arg().asNum("max-include-depth")); DNSResourceRecord rr; string hashed; while (zpt.get(rr)) { diff --git a/pdns/common_startup.cc b/pdns/common_startup.cc index 261de435b1..d40a74a036 100644 --- a/pdns/common_startup.cc +++ b/pdns/common_startup.cc @@ -254,6 +254,7 @@ void declareArguments() ::arg().set("tcp-fast-open", "Enable TCP Fast Open support on the listening sockets, using the supplied numerical value as the queue size")="0"; ::arg().set("max-generate-steps", "Maximum number of $GENERATE steps when loading a zone from a file")="0"; + ::arg().set("max-include-depth", "Maximum number of nested $INCLUDE directives while processing a zone file")="20"; ::arg().setSwitch("upgrade-unknown-types","Transparently upgrade known TYPExxx records. Recommended to keep off, except for PowerDNS upgrades until data sources are cleaned up")="no"; ::arg().setSwitch("svc-autohints", "Transparently fill ipv6hint=auto ipv4hint=auto SVC params with AAAA/A records for the target name of the record (if within the same zone)")="no"; diff --git a/pdns/fuzz_zoneparsertng.cc b/pdns/fuzz_zoneparsertng.cc index 3285860a9d..98e0d5ce38 100644 --- a/pdns/fuzz_zoneparsertng.cc +++ b/pdns/fuzz_zoneparsertng.cc @@ -51,6 +51,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) { ZoneParserTNG zpt(lines, g_rootdnsname); /* limit the number of steps for '$GENERATE' entries */ zpt.setMaxGenerateSteps(10000); + zpt.setMaxIncludes(20); DNSResourceRecord drr; while (zpt.get(drr)) { } diff --git a/pdns/pdns_recursor.cc b/pdns/pdns_recursor.cc index 871dbf7f44..f6e483e733 100644 --- a/pdns/pdns_recursor.cc +++ b/pdns/pdns_recursor.cc @@ -5992,6 +5992,7 @@ int main(int argc, char **argv) ::arg().setSwitch("qname-minimization", "Use Query Name Minimization")="yes"; ::arg().setSwitch("nothing-below-nxdomain", "When an NXDOMAIN exists in cache for a name with fewer labels than the qname, send NXDOMAIN without doing a lookup (see RFC 8020)")="dnssec"; ::arg().set("max-generate-steps", "Maximum number of $GENERATE steps when loading a zone from a file")="0"; + ::arg().set("max-include-depth", "Maximum nested $INCLUDE depth when loading a zone from a file")="20"; ::arg().set("record-cache-shards", "Number of shards in the record cache")="1024"; ::arg().set("refresh-on-ttl-perc", "If a record is requested from the cache and only this % of original TTL remains, refetch") = "0"; diff --git a/pdns/pdnsutil.cc b/pdns/pdnsutil.cc index 8da10873ca..97b21483bb 100644 --- a/pdns/pdnsutil.cc +++ b/pdns/pdnsutil.cc @@ -108,6 +108,7 @@ static void loadMainConfig(const std::string& configdir) ::arg().set("max-signature-cache-entries", "Maximum number of signatures cache entries")=""; ::arg().set("rng", "Specify random number generator to use. Valid values are auto,sodium,openssl,getrandom,arc4random,urandom.")="auto"; ::arg().set("max-generate-steps", "Maximum number of $GENERATE steps when loading a zone from a file")="0"; + ::arg().set("max-include-depth", "Maximum nested $INCLUDE depth when loading a zone from a file")="20"; ::arg().setSwitch("upgrade-unknown-types","Transparently upgrade known TYPExxx records. Recommended to keep off, except for PowerDNS upgrades until data sources are cleaned up")="no"; ::arg().laxFile(configname.c_str()); @@ -1190,6 +1191,7 @@ static int editZone(const DNSName &zone) { cmdline.clear(); ZoneParserTNG zpt(tmpnam, g_rootdnsname); zpt.setMaxGenerateSteps(::arg().asNum("max-generate-steps")); + zpt.setMaxIncludes(::arg().asNum("max-include-depth")); DNSResourceRecord zrr; map, vector > grouped; try { diff --git a/pdns/recursordist/docs/settings.rst b/pdns/recursordist/docs/settings.rst index 8842288a92..804436da57 100644 --- a/pdns/recursordist/docs/settings.rst +++ b/pdns/recursordist/docs/settings.rst @@ -1044,6 +1044,17 @@ Maximum number of incoming requests handled concurrently per tcp connection. This number must be larger than 0 and smaller than 65536 and also smaller than `max-mthreads`. +.. _setting-max-include-depth: + +``max-include-depth`` +---------------------- + +- Integer +- Default: 20 + +Maximum number of nested ``$INCLUDE`` directives while processing a zone file. +Zero mean no ``$INCLUDE`` directives will be accepted. + .. _setting-max-generate-steps: ``max-generate-steps`` diff --git a/pdns/recursordist/rec-zonetocache.cc b/pdns/recursordist/rec-zonetocache.cc index c55168e148..fa3894bb9c 100644 --- a/pdns/recursordist/rec-zonetocache.cc +++ b/pdns/recursordist/rec-zonetocache.cc @@ -218,6 +218,7 @@ void ZoneData::ZoneToCache(const RecZoneToCache::Config& config, uint64_t config DNSResourceRecord drr; ZoneParserTNG zpt(lines, d_zone); zpt.setMaxGenerateSteps(1); + zpt.setMaxIncludes(0); while (zpt.get(drr)) { DNSRecord dr(drr); diff --git a/pdns/reczones.cc b/pdns/reczones.cc index a5430c8515..560f204f89 100644 --- a/pdns/reczones.cc +++ b/pdns/reczones.cc @@ -99,6 +99,7 @@ bool primeHints(time_t ignored) else { ZoneParserTNG zpt(::arg()["hint-file"]); zpt.setMaxGenerateSteps(::arg().asNum("max-generate-steps")); + zpt.setMaxIncludes(::arg().asNum("max-include-depth")); DNSResourceRecord rr; set seenNS; set seenA; @@ -399,6 +400,7 @@ std::shared_ptr parseAuthAndForwards() g_log << Logger::Error << "Parsing authoritative data for zone '" << headers.first << "' from file '" << headers.second << "'" << endl; ZoneParserTNG zpt(headers.second, DNSName(headers.first)); zpt.setMaxGenerateSteps(::arg().asNum("max-generate-steps")); + zpt.setMaxIncludes(::arg().asNum("max-include-depth")); DNSResourceRecord rr; DNSRecord dr; while (zpt.get(rr)) { diff --git a/pdns/rpzloader.cc b/pdns/rpzloader.cc index 5fa63ed22c..03d8228126 100644 --- a/pdns/rpzloader.cc +++ b/pdns/rpzloader.cc @@ -286,6 +286,7 @@ std::shared_ptr loadRPZFromFile(const std::string& fname, std: shared_ptr sr = nullptr; ZoneParserTNG zpt(fname); zpt.setMaxGenerateSteps(::arg().asNum("max-generate-steps")); + zpt.setMaxIncludes(::arg().asNum("max-include-depth")); DNSResourceRecord drr; DNSName domain; while(zpt.get(drr)) { diff --git a/pdns/test-zoneparser_tng_cc.cc b/pdns/test-zoneparser_tng_cc.cc index d435b503a8..751645b228 100644 --- a/pdns/test-zoneparser_tng_cc.cc +++ b/pdns/test-zoneparser_tng_cc.cc @@ -166,20 +166,33 @@ 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: counter out of bounds */ + ZoneParserTNG zp(std::vector({"$GENERATE 4294967296-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: stop out of bounds */ + ZoneParserTNG zp(std::vector({"$GENERATE 0-4294967296/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")); diff --git a/pdns/ws-auth.cc b/pdns/ws-auth.cc index d1d06b037a..118d34c462 100644 --- a/pdns/ws-auth.cc +++ b/pdns/ws-auth.cc @@ -1385,6 +1385,7 @@ static void gatherRecordsFromZone(const std::string& zonestring, vectorfilename, i->name, BP.getDirectory()); zpt.setMaxGenerateSteps(::arg().asNum("max-generate-steps")); + zpt.setMaxIncludes(::arg().asNum("max-include-depth")); DNSResourceRecord rr; obj["name"] = i->name.toString(); diff --git a/pdns/zone2ldap.cc b/pdns/zone2ldap.cc index b1f3434da6..28039bf48e 100644 --- a/pdns/zone2ldap.cc +++ b/pdns/zone2ldap.cc @@ -249,6 +249,7 @@ int main( int argc, char* argv[] ) args.set( "domainid", "Domain ID of the first domain found (incremented afterwards)" ) = "1"; args.set( "metadata-dn", "DN under which to store the domain metadata" ) = ""; args.set( "max-generate-steps", "Maximum number of $GENERATE steps when loading a zone from a file")="0"; + args.set( "max-include-depth", "Maximum nested $INCLUDE depth when loading a zone from a file")="20"; args.parse( argc, argv ); @@ -318,6 +319,7 @@ int main( int argc, char* argv[] ) g_zonename = i.name; ZoneParserTNG zpt(i.filename, i.name, BP.getDirectory()); zpt.setMaxGenerateSteps(args.asNum("max-generate-steps")); + zpt.setMaxIncludes(args.asNum("max-include-depth")); DNSResourceRecord rr; while(zpt.get(rr)) { callback(g_domainid, rr.qname, rr.qtype.toString(), encode_non_ascii(rr.content), rr.ttl); diff --git a/pdns/zone2sql.cc b/pdns/zone2sql.cc index 505fd1dfd7..51f18b91c0 100644 --- a/pdns/zone2sql.cc +++ b/pdns/zone2sql.cc @@ -213,6 +213,7 @@ try ::arg().set("named-conf","Bind 8/9 named.conf to parse")=""; ::arg().set("max-generate-steps", "Maximum number of $GENERATE steps when loading a zone from a file")="0"; + ::arg().set("max-include-depth", "Maximum nested $INCLUDE depth when loading a zone from a file")="20"; ::arg().setCmd("help","Provide a helpful message"); ::arg().setCmd("version","Print the version"); @@ -295,6 +296,7 @@ try ZoneParserTNG zpt(domain.filename, domain.name, BP.getDirectory()); zpt.setMaxGenerateSteps(::arg().asNum("max-generate-steps")); + zpt.setMaxIncludes(::arg().asNum("max-include-depth")); DNSResourceRecord rr; bool seenSOA=false; string comment; diff --git a/pdns/zoneparser-tng.cc b/pdns/zoneparser-tng.cc index 701bbbf91a..ee0741f862 100644 --- a/pdns/zoneparser-tng.cc +++ b/pdns/zoneparser-tng.cc @@ -36,8 +36,9 @@ #include #include #include +#include -static string g_INstr("IN"); +const static string g_INstr("IN"); ZoneParserTNG::ZoneParserTNG(const string& fname, DNSName zname, string reldir, bool upgradeContent): d_reldir(std::move(reldir)), d_zonename(std::move(zname)), d_defaultttl(3600), @@ -57,10 +58,34 @@ ZoneParserTNG::ZoneParserTNG(const vector& zonedata, DNSName zname, boo void ZoneParserTNG::stackFile(const std::string& fname) { - FILE *fp=fopen(fname.c_str(), "r"); - if(!fp) { - std::error_code ec (errno,std::generic_category()); - throw std::system_error(ec, "Unable to open file '"+fname+"': "+stringerror()); + if (d_filestates.size() >= d_maxIncludes) { + std::error_code ec (0, std::generic_category()); + throw std::system_error(ec, "Include limit reached"); + } + int fd = open(fname.c_str(), O_RDONLY, 0); + if (fd == -1) { + int err = errno; + std::error_code ec (err, std::generic_category()); + throw std::system_error(ec, "Unable to open file '" + fname + "': " + stringerror(err)); + } + struct stat st; + if (fstat(fd, &st) == -1) { + int err = errno; + close(fd); + std::error_code ec (err, std::generic_category()); + throw std::system_error(ec, "Unable to stat file '" + fname + "': " + stringerror(err)); + } + if (!S_ISREG(st.st_mode)) { + close(fd); + std::error_code ec (0, std::generic_category()); + throw std::system_error(ec, "File '" + fname + "': not a regular file"); + } + FILE *fp = fdopen(fd, "r"); + if (!fp) { + int err = errno; + close(fd); + std::error_code ec (err, std::generic_category()); + throw std::system_error(ec, "Unable to open file '" + fname + "': " + stringerror(err)); } filestate fs(fp, fname); diff --git a/pdns/zoneparser-tng.hh b/pdns/zoneparser-tng.hh index fd4464738f..1b7562766a 100644 --- a/pdns/zoneparser-tng.hh +++ b/pdns/zoneparser-tng.hh @@ -49,6 +49,10 @@ public: { d_maxGenerateSteps = max; } + void setMaxIncludes(size_t max) + { + d_maxIncludes = max; + } private: bool getLine(); bool getTemplateLine(); @@ -73,6 +77,7 @@ private: std::stack d_filestates; parts_t d_templateparts; size_t d_maxGenerateSteps{0}; + size_t d_maxIncludes{20}; int d_defaultttl; uint32_t d_templatecounter, d_templatestop, d_templatestep; bool d_havedollarttl;