docs: recommend TransactionLimiter for bounding untrusted zone input

dns/transaction.py

@@ -651,3 +651,73 @@ def _get_node(self, name):
     def _origin_information(self):
         # This is only used by _add()
         return self.manager.origin_information()
+
+
+class TransactionSetup:
+    """Abstract base class for additional transaction setup.
+
+    In code which supports it, the setup() method is called on writable transactions
+    just after the transaction is created and before it is used on zone data.  This
+    allows the caller to set additional transaction attributes, e.g. checking.
+    """
+
+    def __init__(self):
+        pass
+
+    def setup(self, txn: Transaction):
+        pass
+
+
+class TooManyChanges(dns.exception.DNSException):
+    """Too many changes"""
+
+
+class TransactionLimiter(TransactionSetup):
+    """Transaction setup that enforces a maximum number of changes in the transaction.
+
+    Each rdataset put or deleted counts as a change.  Each whole name deletion counts
+    as a change.
+
+    The limiter is meant to ensure that reading a zonefile from an untrusted source
+    cannot cause too many changes, e.g. via large $GENERATE statements.  It is not
+    meant to limit the total number of records in a zone or the memory used by the
+    zone.
+    """
+
+    def __init__(self, limit: int):
+        super().__init__()
+        self.limit = limit
+        self.changes: int = 0
+
+    def _check_limit(self):
+        if self.changes >= self.limit:
+            raise TooManyChanges(f"limit is {self.limit} changes")
+
+    def _check_put_rdataset(
+        self, txn: Transaction, name: dns.name.Name, rdataset: dns.rdataset.Rdataset
+    ):
+        self.changes += len(rdataset)
+        self._check_limit()
+
+    def _check_delete_rdataset(
+        self,
+        txn: Transaction,
+        name: dns.name.Name,
+        type: dns.rdatatype.RdataType,
+        covers: dns.rdatatype.RdataType | None,
+    ):
+        self.changes += 1
+        self._check_limit()
+
+    def _check_delete_name(
+        self,
+        txn: Transaction,
+        name: dns.name.Name,
+    ):
+        self.changes += 1
+        self._check_limit()
+
+    def setup(self, txn: Transaction):
+        txn.check_put_rdataset(self._check_put_rdataset)
+        txn.check_delete_rdataset(self._check_delete_rdataset)
+        txn.check_delete_name(self._check_delete_name)

dns/xfr.py

@@ -61,6 +61,7 @@ def __init__(
         rdtype: dns.rdatatype.RdataType = dns.rdatatype.AXFR,
         serial: int | None = None,
         is_udp: bool = False,
+        transaction_setup: dns.transaction.TransactionSetup | None = None,
     ):
         """Initialize an inbound zone transfer.
 
@@ -73,6 +74,12 @@ def __init__(
 
         :param is_udp: Whether UDP is being used for this XFR.
         :type is_udp: bool
+
+        :param transaction_setup: If not ``None``, call the object's setup()
+            method with the just-created writer transaction.  This lets the caller
+            alter the transaction's configuration before it is used, for example adding
+            checking policies.
+        :type transaction_setup: None or dns.transaction.TransactionSetup
         """
         self.txn_manager = txn_manager
         self.txn: dns.transaction.Transaction | None = None
@@ -97,6 +104,7 @@ def __init__(
         self.done = False
         self.expecting_SOA = False
         self.delete_mode = False
+        self.transaction_setup = transaction_setup
 
     def process_message(self, message: dns.message.Message) -> bool:
         """Process one message in the transfer.
@@ -109,6 +117,8 @@ def process_message(self, message: dns.message.Message) -> bool:
         """
         if self.txn is None:
             self.txn = self.txn_manager.writer(not self.incremental)
+            if self.transaction_setup is not None:
+                self.transaction_setup.setup(self.txn)
         rcode = message.rcode()
         if rcode != dns.rcode.NOERROR:
             raise TransferError(rcode)
@@ -228,6 +238,8 @@ def process_message(self, message: dns.message.Message) -> bool:
                 self.delete_mode = False
                 self.txn.rollback()
                 self.txn = self.txn_manager.writer(True)
+                if self.transaction_setup is not None:
+                    self.transaction_setup.setup(self.txn)
                 #
                 # Note we are falling through into the code below
                 # so whatever rdataset this was gets written.

dns/zone.py

@@ -1265,6 +1265,7 @@ def _from_text(
     check_origin: bool = True,
     idna_codec: dns.name.IDNACodec | None = None,
     allow_directives: bool | Iterable[str] = True,
+    transaction_setup: dns.transaction.TransactionSetup | None = None,
 ) -> Zone:
     # See the comments for the public APIs from_text() and from_file() for
     # details.
@@ -1277,6 +1278,8 @@ def _from_text(
         filename = "<string>"
     zone = zone_factory(origin, rdclass, relativize=relativize)
     with zone.writer(True) as txn:
+        if transaction_setup is not None:
+            transaction_setup.setup(txn)
         tok = dns.tokenizer.Tokenizer(text, filename, idna_codec=idna_codec)
         reader = dns.zonefile.Reader(
             tok,
@@ -1308,6 +1311,7 @@ def from_text(
     check_origin: bool = True,
     idna_codec: dns.name.IDNACodec | None = None,
     allow_directives: bool | Iterable[str] = True,
+    transaction_setup: dns.transaction.TransactionSetup | None = None,
 ) -> Zone:
     """Build a zone object from a zone file format string.
 
@@ -1340,6 +1344,11 @@ def from_text(
         non-empty iterable, only the listed directives (including the ``$``)
         are allowed.
     :type allow_directives: bool or Iterable[str]
+    :param transaction_setup: If not ``None``, call the object's setup()
+        method with the just-created writer transaction.  This lets the caller
+        alter the transaction's configuration before it is used, for example adding
+        checking policies.
+    :type transaction_setup: None or dns.transaction.TransactionSetup
     :raises dns.zone.NoSOA: if there is no SOA RRset.
     :raises dns.zone.NoNS: if there is no NS RRset.
     :raises KeyError: if there is no origin node.
@@ -1356,6 +1365,7 @@ def from_text(
         check_origin,
         idna_codec,
         allow_directives,
+        transaction_setup,
     )
 
 
@@ -1370,6 +1380,7 @@ def from_file(
     check_origin: bool = True,
     idna_codec: dns.name.IDNACodec | None = None,
     allow_directives: bool | Iterable[str] = True,
+    transaction_setup: dns.transaction.TransactionSetup | None = None,
 ) -> Zone:
     """Read a zone file and build a zone object.
 
@@ -1403,6 +1414,11 @@ def from_file(
         non-empty iterable, only the listed directives (including the ``$``)
         are allowed.
     :type allow_directives: bool or Iterable[str]
+    :param transaction_setup: If not ``None``, call the object's setup()
+        method with the just-created writer transaction.  This lets the caller
+        alter the transaction's configuration before it is used, for example adding
+        checking policies.
+    :type transaction_setup: None or dns.transaction.TransactionSetup
     :raises dns.zone.NoSOA: if there is no SOA RRset.
     :raises dns.zone.NoNS: if there is no NS RRset.
     :raises KeyError: if there is no origin node.
@@ -1427,6 +1443,7 @@ def from_file(
             check_origin,
             idna_codec,
             allow_directives,
+            transaction_setup,
         )
     assert False  # make mypy happy  lgtm[py/unreachable-statement]
 

doc/zone-make.rst

@@ -6,3 +6,29 @@ Making DNS Zones
 .. autofunction:: dns.zone.from_text
 .. autofunction:: dns.zone.from_file
 .. autofunction:: dns.zone.from_xfr
+
+.. warning::
+
+   These functions build a zone by applying every record they read as a change
+   in a transaction, and by default there is no limit on the number of changes.
+   When the input is untrusted -- for example a user-supplied master file or a
+   "zone import" feature -- a small input can request a very large amount of
+   work: a ``$GENERATE`` directive with a large range, or a deliberately huge
+   zone, can consume excessive memory and CPU.
+
+   When parsing untrusted input, pass a ``dns.transaction.TransactionLimiter``
+   as ``transaction_setup`` to cap the number of changes.  It raises
+   ``dns.transaction.TooManyChanges`` once the limit is exceeded::
+
+       import dns.zone
+       import dns.transaction
+
+       limiter = dns.transaction.TransactionLimiter(100000)
+       try:
+           z = dns.zone.from_file("untrusted.zone", "example.", transaction_setup=limiter)
+       except dns.transaction.TooManyChanges:
+           ...  # reject the oversized input
+
+   Choose a limit appropriate to your application; the same mechanism bounds
+   oversized inbound zone transfers.  The default behaviour remains unlimited
+   for backwards compatibility.

tests/test_zone.py

@@ -33,6 +33,7 @@
 import dns.rdataset
 import dns.rdatatype
 import dns.rrset
+import dns.transaction
 import dns.versioned
 import dns.zone
 from tests.util import here
@@ -1280,6 +1281,17 @@ def testJustificationAndDefaultTTL(self):
         print(example_unicode_justified)
         self.assertEqual(t1, example_unicode_justified)
 
+    def testFromFileHittingLimit(self):
+        limiter = dns.transaction.TransactionLimiter(20)
+        with self.assertRaises(dns.transaction.TooManyChanges):
+            z = dns.zone.from_file(
+                here("example"), "example", transaction_setup=limiter
+            )
+
+    def testFromFileLimitOk(self):
+        limiter = dns.transaction.TransactionLimiter(1000)
+        z = dns.zone.from_file(here("example"), "example", transaction_setup=limiter)
+
 
 class VersionedZoneTestCase(unittest.TestCase):
     zone_factory = dns.versioned.Zone