Bound the declared payload length and the domain on read and write

README.md

@@ -28,6 +28,47 @@ is left to the caller.
 Version 3 is the current version produced for new OWIDs. Versions 1 and 2 are
 deprecated and are supported for reading existing data only.
 
+## Payload size and application limits
+
+The OWID wire format stores the payload length as an unsigned 32 bit value,
+so a payload from zero through 4,294,967,295 bytes is structurally valid. The
+format defines no smaller payload limit. The null-terminated domain carries
+no length of its own, so the protocol alone is not an application input limit
+for the complete envelope.
+
+This package validates that the declared payload length agrees with the bytes
+present before it sizes or copies the payload. A large declaration without
+the corresponding bytes is malformed and is rejected without allocating the
+declared size. A matching large payload is not malformed merely because it is
+large, and parsing work and memory use scale with the bytes actually present.
+
+The domain ends at a zero terminator rather than at a declared length, so a
+buffer whose terminator is missing or corrupted would otherwise be walked to
+its end. This package stops that walk at `MAXIMUM_DOMAIN_LENGTH`, which
+`owid/io.py` derives from the size limit in RFC 1035 section 2.3.4, and
+refuses the buffer there. The cost of a domain field an attacker sized is
+therefore fixed by that constant rather than by the length of the input, and
+no domain a name server would accept is affected.
+
+The same maximum binds the write, because a library that emits something it
+cannot read moves the fault to the consumer. A `Creator` refuses a domain
+longer than `MAXIMUM_DOMAIN_LENGTH` when the caller supplies it, before any
+signing work is done, and the writer refuses one that reached an OWID by any
+other route when the OWID is serialised. Both raise `OwidError` naming the
+maximum, as the parse does.
+
+The in-memory APIs remain subject to Python object, address-space and
+available-memory limits. Applications accepting untrusted OWIDs must choose
+limits suitable for their use case and enforce them before buffering the
+binary form or decoding Base64. An implementation capacity failure or an
+application policy rejection is distinct from an invalid OWID.
+
+For transport input, limit the complete HTTP body or encoded envelope; allow
+for the domain and other OWID fields as well as the payload. After parsing,
+`len(owid.payload)` reports the actual payload size without another copy and
+can be used for downstream policy. The parser cannot choose either limit on
+behalf of the application.
+
 ## Installation
 
 The package targets Python 3.9 and later and depends on the

owid/creator.py

@@ -27,7 +27,7 @@
 
 from .crypto import Crypto
 from .error import OwidError
-from .io import SIGNATURE_LENGTH
+from .io import MAXIMUM_DOMAIN_LENGTH, SIGNATURE_LENGTH
 from .owid import Owid
 from .version import DEFAULT_VERSION
 
@@ -62,11 +62,22 @@ def __init__(self, domain: str, crypto: Crypto) -> None:
         """Creates a new creator for the domain using the crypto instance for
         signing.
 
-        Raises OwidError if the domain is empty or whitespace, or the crypto
-        instance can not sign.
+        Raises OwidError if the domain is empty or whitespace, longer than
+        the published maximum, or the crypto instance can not sign.
+
+        The domain is checked here, where the caller supplies it, rather than
+        only when an OWID is written, so a creator that could only produce
+        OWIDs this library refuses to read never exists and no signing work
+        is done before the refusal.
         """
         if domain is None or domain.strip() == "":
             raise OwidError("domain '{0}' is not valid".format(domain))
+        if len(domain.encode("utf-8")) > MAXIMUM_DOMAIN_LENGTH:
+            raise OwidError(
+                "domain is longer than the '{0}' character maximum".format(
+                    MAXIMUM_DOMAIN_LENGTH
+                )
+            )
         if not crypto.can_sign():
             raise OwidError("instance of Crypto cannot be used to generate a signature")
         self._domain = domain
@@ -77,8 +88,9 @@ def from_configuration(cls, configuration: Configuration) -> "Creator":
         """Creates a new creator from configuration containing the domain and
         the private key PEM.
 
-        Raises OwidError if the domain is empty or whitespace, or the private
-        key PEM is not valid.
+        Raises OwidError if the domain is empty or whitespace, the domain is
+        longer than the published maximum, or the private key PEM is not
+        valid.
         """
         crypto = Crypto.new_sign_only(configuration.private_key)
         return cls(configuration.domain, crypto)

owid/io.py

@@ -36,6 +36,15 @@
 #: number of hours or minutes after this instant.
 BASE_DATE = datetime(2020, 1, 1, tzinfo=timezone.utc)
 
+#: The longest creator domain the reader will accept, in characters. RFC 1035
+#: section 2.3.4, "Size limits", restricts the total length of a domain name,
+#: being the label octets and the label length octets, to 255 octets or less.
+#: An OWID stores the presentation form, the text "example.com", where the
+#: dots stand in for the label length octets and the root label has no text at
+#: all, so two of those 255 octets have no text equivalent and the limit on
+#: the text is two fewer. A domain is ASCII, so a character is a byte here.
+MAXIMUM_DOMAIN_LENGTH = 255 - 2
+
 
 class Reader:
     """Sequential reader over a byte buffer."""
@@ -62,26 +71,61 @@ def read_bytes(self, count: int) -> bytes:
         return value
 
     def read_string(self) -> str:
-        """Reads bytes up to the null terminator and decodes them as UTF-8."""
-        remaining = self._buffer[self._position:]
-        terminator = remaining.find(0)
+        """Reads bytes up to the null terminator and decodes them as UTF-8.
+
+        The only null terminated string in an OWID is the creator domain, and
+        a domain has a published maximum length, so the search for the
+        terminator stops after MAXIMUM_DOMAIN_LENGTH bytes rather than running
+        to the end of the buffer. A buffer whose terminator is missing or
+        corrupted is refused as soon as that window is exhausted, so the work
+        a hostile buffer can ask for is fixed by the constant rather than
+        growing with the length of the input.
+        """
+        window_end = self._position + MAXIMUM_DOMAIN_LENGTH + 1
+        terminator = self._buffer.find(b"\0", self._position, window_end)
         if terminator < 0:
-            raise OwidError("buffer ended before the OWID was complete")
+            if len(self._buffer) < window_end:
+                raise OwidError("buffer ended before the OWID was complete")
+            raise OwidError(
+                "domain is longer than the '{0}' character maximum".format(
+                    MAXIMUM_DOMAIN_LENGTH
+                )
+            )
         try:
-            value = remaining[:terminator].decode("utf-8")
+            value = self._buffer[self._position:terminator].decode("utf-8")
         except UnicodeDecodeError:
             raise OwidError("domain bytes are not valid UTF-8")
-        self._position += terminator + 1
+        self._position = terminator + 1
         return value
 
     def read_u32(self) -> int:
         """Reads an unsigned 32 bit little endian integer."""
-        return struct.unpack("<I", self.read_bytes(4))[0]
+        if self._position + 4 > len(self._buffer):
+            raise OwidError("buffer ended before the OWID was complete")
+        value = struct.unpack_from("<I", self._buffer, self._position)[0]
+        self._position += 4
+        return value
 
     def read_byte_array(self) -> bytes:
-        """Reads a byte array prefixed with its length as an unsigned 32 bit
-        integer."""
+        """Reads the payload, being a byte array prefixed with its length as
+        an unsigned 32 bit integer.
+
+        The length is whatever the sender declared, so it is checked against
+        the bytes actually present before anything is sized by it. A valid
+        OWID is the declared payload followed by the signature and nothing
+        else, so the length must equal the bytes remaining less the signature
+        length, and any other length, short or long, is refused here. The
+        same check refuses a signature shorter than 64 bytes and any byte
+        after the signature, which until 28 August 2026 this reader ignored.
+        """
         count = self.read_u32()
+        remaining = len(self._buffer) - self._position
+        if remaining != count + SIGNATURE_LENGTH:
+            raise OwidError(
+                "OWID payload length '{0}' does not match the '{1}' bytes "
+                "present, of which the final '{2}' must be the "
+                "signature".format(count, remaining, SIGNATURE_LENGTH)
+            )
         return self.read_bytes(count)
 
     def read_signature(self) -> bytes:
@@ -111,10 +155,23 @@ def write_string(buffer: bytearray, value: str) -> None:
 
     The string must not contain a null character because that would conflict
     with the terminator.
+
+    The only string written this way is the creator domain, and the reader
+    refuses a domain longer than MAXIMUM_DOMAIN_LENGTH, so a longer one is
+    refused here as well and the library never emits an OWID that it would
+    then refuse to read. The length compared is the encoded bytes, because
+    those are what the reader walks, and for the ASCII a domain is made of
+    they are the same count as the characters.
     """
     encoded = value.encode("utf-8")
     if 0 in encoded:
         raise OwidError("domain '{0}' is not valid".format(value))
+    if len(encoded) > MAXIMUM_DOMAIN_LENGTH:
+        raise OwidError(
+            "domain is longer than the '{0}' character maximum".format(
+                MAXIMUM_DOMAIN_LENGTH
+            )
+        )
     buffer.extend(encoded)
     buffer.append(0)
 

owid/owid.py

@@ -94,8 +94,11 @@ def from_base64(cls, value: str) -> "Owid":
     def from_byte_array(cls, buffer: bytes) -> "Owid":
         """Creates an OWID from its binary form.
 
-        Raises OwidError if the first byte is not a known version or the
-        buffer is too short for the remaining fields.
+        The buffer must hold exactly one OWID, ending with the 64 byte
+        signature. Raises OwidError if the first byte is not a known
+        version, the buffer is too short for the remaining fields, or the
+        declared payload length does not leave exactly the signature at the
+        end of the buffer.
         """
         reader = io.Reader(bytes(buffer))
         return cls._from_reader(reader)