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

README.md

@@ -36,6 +36,46 @@ Fetching a creator public key over HTTP is out of scope. The
 `verifyWithPublicKey` method accepts a public key PEM that the caller has
 already obtained, so any HTTP client can supply it.
 
+## 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 before it either, so the protocol alone is not an application input
+limit for the complete envelope.
+
+This library validates that the declared payload length agrees with the bytes
+present before it extracts 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 is read the same way. Because nothing declares its length, the
+search for its terminator stops at the greatest number of characters a domain
+name can hold, which RFC 1035 section 2.3.4 fixes for the presentation form
+this library stores. A domain with no terminator, or one longer than a domain
+name may be, is rejected for a cost set by that maximum rather than by the
+length of the buffer.
+
+The same maximum binds the write, so this library cannot produce an OWID it
+would then refuse to read. A `Creator` refuses a domain longer than the
+maximum when the domain is supplied, which is the earliest point the caller
+can be told, and the serialization refuses one as well, so a domain that
+reaches the `Owid` domain field by any other route is caught before the
+signature is calculated.
+
+The in-memory APIs remain subject to PHP string, platform, 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,
+`strlen($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
 
 Require the package with Composer.

src/Creator.php

@@ -35,16 +35,25 @@ final class Creator
 
     /**
      * Creates a new creator for the domain using the crypto instance for
-     * signing.
+     * signing. The domain is bounded here, at the earliest point the caller
+     * can be told, so a creator configured with a domain longer than a
+     * domain name can hold is refused when the domain is supplied rather
+     * than when an OWID is later serialized. The check comes before the
+     * crypto instance is looked at, so nothing is signed with a domain this
+     * same library would then refuse to read.
      *
-     * @throws OwidException when the domain is empty or whitespace, or the
+     * @throws OwidException when the domain is empty or whitespace, is
+     *                       longer than a domain name can hold, or the
      *                       crypto instance can not sign.
      */
     public function __construct(string $domain, Crypto $crypto)
     {
         if (trim($domain) === '') {
             throw OwidException::invalidDomain($domain);
         }
+        if (strlen($domain) > OwidException::MAXIMUM_DOMAIN_LENGTH) {
+            throw OwidException::domainTooLong();
+        }
         if (!$crypto->canSign()) {
             throw OwidException::keyMissing('generate a signature');
         }
@@ -55,7 +64,8 @@ public function __construct(string $domain, Crypto $crypto)
     /**
      * Creates a new creator from the domain and the private key PEM provided.
      *
-     * @throws OwidException when the domain is empty or whitespace, or the
+     * @throws OwidException when the domain is empty or whitespace, is
+     *                       longer than a domain name can hold, or the
      *                       private key PEM is not valid.
      */
     public static function fromConfiguration(string $domain, string $privatePem): self

src/Io.php

@@ -50,11 +50,13 @@ public static function baseDate(): DateTimeImmutable
      * buffer because each element accessed by offset is a single byte.
      */
     private string $buffer;
+    private int $length;
     private int $position;
 
     public function __construct(string $buffer)
     {
         $this->buffer = $buffer;
+        $this->length = strlen($buffer);
         $this->position = 0;
     }
 
@@ -65,7 +67,7 @@ public function __construct(string $buffer)
      */
     public function readByte(): int
     {
-        if ($this->position >= strlen($this->buffer)) {
+        if ($this->position >= $this->length) {
             throw OwidException::unexpectedEndOfBuffer();
         }
         $value = ord($this->buffer[$this->position]);
@@ -80,31 +82,50 @@ public function readByte(): int
      */
     public function readBytes(int $count): string
     {
-        if ($count < 0 || $this->position + $count > strlen($this->buffer)) {
+        if ($count < 0 || $this->position + $count > $this->length) {
             throw OwidException::unexpectedEndOfBuffer();
         }
         $value = substr($this->buffer, $this->position, $count);
         $this->position += $count;
         return $value;
     }
 
+    /**
+     * Returns the number of unread bytes, so a top-level decoder can require
+     * EOF while a framed reader can deliberately leave following data.
+     */
+    public function remaining(): int
+    {
+        return $this->length - $this->position;
+    }
+
     /**
      * Reads bytes until the null terminator and returns them as a string. The
-     * terminator is consumed but not returned.
+     * terminator is consumed but not returned. The only such string in an
+     * OWID is the creator domain, and the terminator is whatever the sender
+     * wrote, so the search for it stops after the greatest number of
+     * characters a domain name can hold rather than running to the end of the
+     * buffer. A buffer with no terminator therefore costs the bound and not
+     * its own length. strcspn is used because it takes the window as an
+     * argument and so examines no more bytes than the window, whereas strpos
+     * would search the rest of the buffer.
      *
-     * @throws OwidException when no terminator is found.
+     * @throws OwidException when the domain has no terminator within the
+     *                       characters a domain name can hold, or the buffer
+     *                       ends before the terminator.
      */
     public function readString(): string
     {
-        $terminator = strpos($this->buffer, "\0", $this->position);
-        if ($terminator === false) {
+        $maximum = OwidException::MAXIMUM_DOMAIN_LENGTH;
+        $count = strcspn($this->buffer, "\0", $this->position, $maximum + 1);
+        if ($count > $maximum) {
+            throw OwidException::domainTooLong();
+        }
+        $terminator = $this->position + $count;
+        if ($terminator >= $this->length) {
             throw OwidException::unexpectedEndOfBuffer();
         }
-        $value = substr(
-            $this->buffer,
-            $this->position,
-            $terminator - $this->position
-        );
+        $value = substr($this->buffer, $this->position, $count);
         $this->position = $terminator + 1;
         return $value;
     }
@@ -116,15 +137,21 @@ public function readString(): string
      */
     public function readUint32(): int
     {
-        $bytes = $this->readBytes(4);
+        if ($this->position + 4 > $this->length) {
+            throw OwidException::unexpectedEndOfBuffer();
+        }
         /** @var array{1: int} $unpacked */
-        $unpacked = unpack('V', $bytes);
+        $unpacked = unpack('V', $this->buffer, $this->position);
+        $this->position += 4;
         return $unpacked[1];
     }
 
     /**
      * Reads a byte array prefixed with its length as an unsigned 32 bit
-     * integer.
+     * integer. The count is bounded by the bytes present in readBytes, so
+     * nothing is sized by the declared number alone. The OWID payload is
+     * read with readPayload instead, because the payload must also be
+     * followed by the fixed-length signature.
      *
      * @throws OwidException when the buffer is too short.
      */
@@ -134,6 +161,27 @@ public function readByteArray(): string
         return $this->readBytes($count);
     }
 
+    /**
+     * Reads the length prefixed payload of an OWID, which must be followed
+     * by the signature. The count is whatever the sender
+     * declared, so it is checked against the bytes actually present before
+     * anything is sized by it. The count must leave at least the signature;
+     * a public reader consumes one OWID and leaves following framed bytes,
+     * while top-level byte-array parsing separately requires EOF.
+     *
+     * @throws OwidException when the declared length does not leave a
+     *                       complete signature after the payload.
+     */
+    public function readPayload(): string
+    {
+        $count = $this->readUint32();
+        $present = $this->length - $this->position;
+        if ($count + OwidException::SIGNATURE_LENGTH > $present) {
+            throw OwidException::payloadLengthMismatch($count, $present);
+        }
+        return $this->readBytes($count);
+    }
+
     /**
      * Reads the fixed length signature.
      *
@@ -177,16 +225,29 @@ public static function writeByte(string &$buffer, int $value): void
     }
 
     /**
-     * Writes the string followed by the null terminator. The string must not
-     * contain a null character as that would conflict with the terminator.
+     * Writes the string followed by the null terminator. The only such string
+     * in an OWID is the creator domain. The value must not contain a null
+     * character as that would conflict with the terminator, and must not be
+     * longer than the greatest number of characters a domain name can hold,
+     * because readString stops looking for the terminator at that bound and
+     * would refuse anything longer. Without this the library could write an
+     * OWID it then refused to read, and the fault would land on whoever read
+     * it rather than on the creator that caused it. This is the later of the
+     * two write side checks, and it catches a domain that reached the OWID
+     * by some route other than the creator, such as the public domain field
+     * being assigned directly.
      *
-     * @throws OwidException when the value contains a null byte.
+     * @throws OwidException when the value contains a null byte, or is
+     *                       longer than a domain name can hold.
      */
     public static function writeString(string &$buffer, string $value): void
     {
         if (strpos($value, "\0") !== false) {
             throw OwidException::invalidDomain($value);
         }
+        if (strlen($value) > OwidException::MAXIMUM_DOMAIN_LENGTH) {
+            throw OwidException::domainTooLong();
+        }
         $buffer .= $value . "\0";
     }
 

src/Owid.php

@@ -83,21 +83,33 @@ public static function fromBase64(string $value): self
     }
 
     /**
-     * Creates an OWID from its binary form.
+     * Creates an OWID from its complete binary form. Bytes missing from or
+     * following the envelope are refused.
      *
-     * @throws OwidException when the version is unknown or the buffer is too
-     *                       short for the remaining fields.
+     * @throws OwidException when the version is unknown, the buffer is too
+     *                       short for the remaining fields, or the declared
+     *                       payload length does not match the bytes present.
      */
     public static function fromByteArray(string $buffer): self
     {
-        return self::fromReader(new Io($buffer));
+        $reader = new Io($buffer);
+        $owid = self::fromReader($reader);
+        if ($reader->remaining() !== 0) {
+            throw new OwidException(
+                "OWID contains '" . $reader->remaining() .
+                "' bytes after the envelope"
+            );
+        }
+        return $owid;
     }
 
     /**
-     * Creates an OWID by reading the next fields from the reader.
+     * Creates an OWID by reading its next fields from the reader. Bytes after
+     * the signature are left for a caller that frames multiple values.
      *
-     * @throws OwidException when the version is unknown or the buffer is too
-     *                       short.
+     * @throws OwidException when the version is unknown, the buffer is too
+     *                       short, or the declared payload length does not
+     *                       match the bytes present.
      */
     public static function fromReader(Io $reader): self
     {
@@ -109,7 +121,7 @@ public static function fromReader(Io $reader): self
         }
         $owid->domain = $reader->readString();
         $owid->date = $reader->readDate($version);
-        $owid->payload = $reader->readByteArray();
+        $owid->payload = $reader->readPayload();
         $owid->signature = $reader->readSignature();
         return $owid;
     }

src/OwidException.php

@@ -34,6 +34,18 @@ final class OwidException extends Exception
      */
     public const SIGNATURE_LENGTH = 64;
 
+    /**
+     * The greatest number of characters an OWID domain can hold. RFC 1035
+     * section 2.3.4, "Size limits", restricts the total length of a domain
+     * name, counting label octets and label length octets, to 255 octets or
+     * less. That 255 is the wire format, which spends one length octet on
+     * every label and one zero octet on the root, whereas OWID stores the
+     * presentation form, the text "example.com", where the dots stand in for
+     * the label length octets and the root has no text at all, so exactly
+     * two of those 255 octets have no character here and the limit is 253.
+     */
+    public const MAXIMUM_DOMAIN_LENGTH = 253;
+
     /**
      * The version byte is not one supported by this implementation.
      */
@@ -79,6 +91,26 @@ public static function invalidDomain(string $domain): self
         return new self("domain '$domain' is not valid");
     }
 
+    /**
+     * The domain is longer than the greatest number of characters a domain
+     * name can hold. Both halves of the library raise this, so both report
+     * the one condition the one way. On a read the domain field has no
+     * terminator within that many characters, so whatever the field holds
+     * runs past the bound, and on a write the value handed in is longer
+     * than the bound. The domain is not named because on a read the bytes
+     * are whatever the sender wrote and there may be no end to them, and
+     * because writeString cannot tell which of the two routes a value
+     * arrived by.
+     */
+    public static function domainTooLong(): self
+    {
+        $maximum = self::MAXIMUM_DOMAIN_LENGTH;
+        return new self(
+            "OWID domain is longer than the '$maximum' characters a domain " .
+            "name can hold"
+        );
+    }
+
     /**
      * The date can not be represented in the encoding used by the version.
      */
@@ -89,6 +121,22 @@ public static function dateOutOfRange(): self
         );
     }
 
+    /**
+     * The declared payload length does not leave a complete signature after
+     * the payload. The declared value is whatever the sender wrote, so it is
+     * named alongside the bytes that were actually present.
+     */
+    public static function payloadLengthMismatch(
+        int $declared,
+        int $present
+    ): self {
+        $signature = self::SIGNATURE_LENGTH;
+        return new self(
+            "OWID payload length '$declared' exceeds the '$present' bytes " .
+            "present, which must also contain the '$signature' byte signature"
+        );
+    }
+
     /**
      * The payload is larger than the unsigned 32 bit length prefix allows.
      */