From 0ab75c25be236f26f94a8ba28f56aa48c0551f4f Mon Sep 17 00:00:00 2001 From: Aidan Woods Date: Thu, 7 Oct 2021 20:09:25 +0100 Subject: [PATCH] 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. --- src/Exception/ExceptionCode.php | 3 +++ src/Protocol/Version1.php | 16 ++++++++++++++++ src/Protocol/Version2.php | 15 +++++++++++++++ src/Protocol/Version3.php | 16 ++++++++++++++++ src/Protocol/Version4.php | 15 +++++++++++++++ 5 files changed, 65 insertions(+) diff --git a/src/Exception/ExceptionCode.php b/src/Exception/ExceptionCode.php index f74aef2..9f2974a 100644 --- 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 index ffafb5b..c5866b1 100644 --- 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 index 1f09948..91a7255 100644 --- 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 index 9794bfe..61fbb28 100644 --- 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 index 1d9bf2e..a30fc33 100644 --- 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,