Error if received message is too short

Not making this check results in negative lengths, offsets, or both being used in the destructuring of the payload. This can create scenarios where certain payloads of different lengths are indistinguishable, and where e.g. the exact same bytes are used in both parts of the nonce, ciphertext, and mac. This early abort does not interact with the plaintext and so no useful information should be disclosed by this.
paragonie · Oct 7, 2021 · 0ab75c2 · 0ab75c2
1 parent 6e3ce7f
commit 0ab75c2
Show file tree

Hide file tree

Showing 5 changed files with 65 additions and 0 deletions.
diff --git a/src/Exception/ExceptionCode.php b/src/Exception/ExceptionCode.php
@@ -38,6 +38,7 @@ abstract class ExceptionCode
     const INVOKED_RAW_ON_MULTIKEY          = 0x3142EE1C;
     const KEY_NOT_IN_KEYRING               = 0x3142EE1D;
     const UNDEFINED_PROPERTY               = 0x3142EE1E;
+    const INVALID_MESSAGE_LENGTH           = 0x3142EE1F;
 
     /**
      * @param int $code
@@ -92,6 +93,8 @@ public static function explainErrorCode(int $code): string
                 return "The key you requested is not in this keyring.";
             case self::UNDEFINED_PROPERTY:
                 return "An expected property was not defined at runtime.";
+            case self::INVALID_MESSAGE_LENGTH:
+                return "The recieved PASETO was too short to be valid.";
 
             default:
                 return 'Unknown error code';

diff --git a/src/Protocol/Version1.php b/src/Protocol/Version1.php
@@ -347,6 +347,14 @@ public static function verify(
         // PASETO Version 1 - Verify - Step 4:
         $decoded = Base64UrlSafe::decode(Binary::safeSubstr($signMsg, $headerLength));
         $len = Binary::safeStrlen($decoded);
+
+        if ($len <= self::SIGN_SIZE) {
+            throw new PasetoException(
+                'Invalid message length.',
+                ExceptionCode::INVALID_MESSAGE_LENGTH
+            );
+        }
+
         $message = Binary::safeSubstr($decoded, 0, $len - self::SIGN_SIZE);
         $signature = Binary::safeSubstr($decoded, $len - self::SIGN_SIZE);
 
@@ -473,6 +481,14 @@ public static function aeadDecrypt(
             );
         }
         $len = Binary::safeStrlen($decoded);
+
+        if ($len <= self::NONCE_SIZE + self::MAC_SIZE) {
+            throw new PasetoException(
+                'Invalid message length.',
+                ExceptionCode::INVALID_MESSAGE_LENGTH
+            );
+        }
+
         $nonce = Binary::safeSubstr($decoded, 0, self::NONCE_SIZE);
         $ciphertext = Binary::safeSubstr(
             $decoded,

diff --git a/src/Protocol/Version2.php b/src/Protocol/Version2.php
@@ -333,6 +333,13 @@ public static function verify(
         $decoded = Base64UrlSafe::decode(Binary::safeSubstr($signMsg, $headerLength));
         $len = Binary::safeStrlen($decoded);
 
+        if ($len <= SODIUM_CRYPTO_SIGN_BYTES) {
+            throw new PasetoException(
+                'Invalid message length.',
+                ExceptionCode::INVALID_MESSAGE_LENGTH
+            );
+        }
+
         // PASETO Version 2 - Verify - Step 4:
         // Separate the decoded bundle into the message and signature.
         $message = Binary::safeSubstr(
@@ -459,6 +466,14 @@ public static function aeadDecrypt(
             );
         }
         $len = Binary::safeStrlen($decoded);
+
+        if ($len <= SODIUM_CRYPTO_AEAD_XCHACHA20POLY1305_IETF_NPUBBYTES) {
+            throw new PasetoException(
+                'Invalid message length.',
+                ExceptionCode::INVALID_MESSAGE_LENGTH
+            );
+        }
+
         $nonce = Binary::safeSubstr(
             $decoded,
             0,

diff --git a/src/Protocol/Version3.php b/src/Protocol/Version3.php
@@ -359,6 +359,14 @@ public static function verify(
         // PASETO Version 3 - Verify - Step 4:
         $decoded = Base64UrlSafe::decode(Binary::safeSubstr($signMsg, $headerLength));
         $len = Binary::safeStrlen($decoded);
+
+        if ($len <= self::SIGN_SIZE) {
+            throw new PasetoException(
+                'Invalid message length.',
+                ExceptionCode::INVALID_MESSAGE_LENGTH
+            );
+        }
+
         $message = Binary::safeSubstr($decoded, 0, $len - self::SIGN_SIZE);
         $signature = Binary::safeSubstr($decoded, $len - self::SIGN_SIZE);
 
@@ -502,6 +510,14 @@ public static function aeadDecrypt(
             );
         }
         $len = Binary::safeStrlen($decoded);
+
+        if ($len <= self::NONCE_SIZE + self::MAC_SIZE) {
+            throw new PasetoException(
+                'Invalid message length.',
+                ExceptionCode::INVALID_MESSAGE_LENGTH
+            );
+        }
+
         $nonce = Binary::safeSubstr($decoded, 0, self::NONCE_SIZE);
         $ciphertext = Binary::safeSubstr(
             $decoded,

diff --git a/src/Protocol/Version4.php b/src/Protocol/Version4.php
@@ -329,6 +329,13 @@ public static function verify(
         $decoded = Base64UrlSafe::decode(Binary::safeSubstr($signMsg, $headerLength));
         $len = Binary::safeStrlen($decoded);
 
+        if ($len <= SODIUM_CRYPTO_SIGN_BYTES) {
+            throw new PasetoException(
+                'Invalid message length.',
+                ExceptionCode::INVALID_MESSAGE_LENGTH
+            );
+        }
+
         // Separate the decoded bundle into the message and signature.
         $message = Binary::safeSubstr(
             $decoded,
@@ -465,6 +472,14 @@ public static function aeadDecrypt(
             );
         }
         $len = Binary::safeStrlen($decoded);
+
+        if ($len <= self::NONCE_SIZE + self::MAC_SIZE) {
+            throw new PasetoException(
+                'Invalid message length.',
+                ExceptionCode::INVALID_MESSAGE_LENGTH
+            );
+        }
+
         $nonce = Binary::safeSubstr($decoded, 0, self::NONCE_SIZE);
         $ciphertext = Binary::safeSubstr(
             $decoded,