From aa26c506598e09d29556de875f2fa0cb864e042c Mon Sep 17 00:00:00 2001 From: Tim van Dijen Date: Thu, 25 Apr 2024 16:22:10 +0200 Subject: [PATCH] Improve parsing of base64-encoded strings --- src/CryptoEncoding/PEM.php | 2 +- src/CryptoEncoding/PEMBundle.php | 5 ++++- src/XML/EncryptedElementTrait.php | 2 +- src/XML/SignedElementTrait.php | 4 ++-- src/XML/xenc/EncryptedKey.php | 2 +- tests/Alg/Encryption/AESEncryptionTest.php | 2 +- tests/Alg/Encryption/TripleDesEncryptionTest.php | 2 +- tests/Alg/KeyTransport/RSAKeyTransportTest.php | 6 +++--- 8 files changed, 14 insertions(+), 11 deletions(-) diff --git a/src/CryptoEncoding/PEM.php b/src/CryptoEncoding/PEM.php index b6093a29..53b46cd5 100644 --- a/src/CryptoEncoding/PEM.php +++ b/src/CryptoEncoding/PEM.php @@ -91,7 +91,7 @@ public static function fromString(string $str): self $payload = preg_replace('/\s+/', '', $match[2]); $data = base64_decode($payload, true); - if ($data === false) { + if (empty($data)) { throw new UnexpectedValueException('Failed to decode PEM data.'); } diff --git a/src/CryptoEncoding/PEMBundle.php b/src/CryptoEncoding/PEMBundle.php index a0f1330e..551ef951 100644 --- a/src/CryptoEncoding/PEMBundle.php +++ b/src/CryptoEncoding/PEMBundle.php @@ -8,6 +8,7 @@ use Countable; use IteratorAggregate; use LogicException; +use SimpleSAML\Assert\Assert; use SimpleSAML\XMLSecurity\Exception\IOException; use UnexpectedValueException; @@ -76,8 +77,10 @@ public static function fromString(string $str): self $pems = array_map( function ($match) { $payload = preg_replace('/\s+/', '', $match[2]); + Assert::stringPlausibleBase64($payload); + $data = base64_decode($payload, true); - if (false === $data) { + if (empty($data)) { throw new UnexpectedValueException( 'Failed to decode PEM data.' ); diff --git a/src/XML/EncryptedElementTrait.php b/src/XML/EncryptedElementTrait.php index b03337d0..d68463bc 100644 --- a/src/XML/EncryptedElementTrait.php +++ b/src/XML/EncryptedElementTrait.php @@ -140,7 +140,7 @@ protected function decryptData(EncryptionAlgorithmInterface $decryptor): string throw new InvalidArgumentException('Decryption algorithm does not match EncryptionMethod.'); } - return $decryptor->decrypt(base64_decode($encData->getCipherData()->getCipherValue()->getContent())); + return $decryptor->decrypt(base64_decode($encData->getCipherData()->getCipherValue()->getContent(), true)); } diff --git a/src/XML/SignedElementTrait.php b/src/XML/SignedElementTrait.php index 11d93f66..ca59d29e 100644 --- a/src/XML/SignedElementTrait.php +++ b/src/XML/SignedElementTrait.php @@ -151,7 +151,7 @@ private function validateReference(SignedInfo $signedInfo): SignedElementInterfa $data = XML::processTransforms($reference->getTransforms(), $xml); $digest = Security::hash($reference->getDigestMethod()->getAlgorithm(), $data, false); - if (Security::compareStrings($digest, base64_decode($reference->getDigestValue()->getRawContent())) !== true) { + if (Security::compareStrings($digest, base64_decode($reference->getDigestValue()->getRawContent(), true)) !== true) { throw new SignatureVerificationFailedException('Failed to verify signature.'); } @@ -187,7 +187,7 @@ private function verifyInternal(SignatureAlgorithmInterface $verifier): SignedEl if ( $verifier?->verify( $c14nSignedInfo, // the canonicalized ds:SignedInfo element (plaintext) - base64_decode($this->signature->getSignatureValue()->getRawContent()), // the actual signature + base64_decode($this->signature->getSignatureValue()->getRawContent(), true), // the actual signature ) ) { /* diff --git a/src/XML/xenc/EncryptedKey.php b/src/XML/xenc/EncryptedKey.php index dbdbd75a..76754a4e 100644 --- a/src/XML/xenc/EncryptedKey.php +++ b/src/XML/xenc/EncryptedKey.php @@ -108,7 +108,7 @@ public function decrypt(EncryptionAlgorithmInterface $decryptor): string InvalidArgumentException::class, ); - return $decryptor->decrypt(base64_decode($cipherValue->getContent())); + return $decryptor->decrypt(base64_decode($cipherValue->getContent(), true)); } diff --git a/tests/Alg/Encryption/AESEncryptionTest.php b/tests/Alg/Encryption/AESEncryptionTest.php index 8e8c3e5c..638db70b 100644 --- a/tests/Alg/Encryption/AESEncryptionTest.php +++ b/tests/Alg/Encryption/AESEncryptionTest.php @@ -52,7 +52,7 @@ public function testEncrypt(): void public function testDecrypt(): void { $ciphertext = "r0YRkEixBnAKU032/ux7avHcVTH1CIIyKaPA2qr4KlIs0LVZp5CuwQKRRi6lji4cnaFbH4jETtJhMSEfbpSdvg=="; - $plaintext = self::$algo->decrypt(base64_decode($ciphertext)); + $plaintext = self::$algo->decrypt(base64_decode($ciphertext, true)); $this->assertEquals("\n \n\tHello, World!\n \n", $plaintext); } } diff --git a/tests/Alg/Encryption/TripleDesEncryptionTest.php b/tests/Alg/Encryption/TripleDesEncryptionTest.php index ffc1a679..6a5d245e 100644 --- a/tests/Alg/Encryption/TripleDesEncryptionTest.php +++ b/tests/Alg/Encryption/TripleDesEncryptionTest.php @@ -52,7 +52,7 @@ public function testEncrypt(): void public function testDecrypt(): void { $ciphertext = "D+3dKq7MFK7U+8bqdlyRcvO12JV5Lahl5ALhF5eJXSfi+cbYKRbkRjvJsMKPp2Mk"; - $plaintext = self::$algo->decrypt(base64_decode($ciphertext)); + $plaintext = self::$algo->decrypt(base64_decode($ciphertext, true)); $this->assertEquals("\n \n\tHello, World!\n \n", $plaintext); } } diff --git a/tests/Alg/KeyTransport/RSAKeyTransportTest.php b/tests/Alg/KeyTransport/RSAKeyTransportTest.php index d79b1e6f..470abea9 100644 --- a/tests/Alg/KeyTransport/RSAKeyTransportTest.php +++ b/tests/Alg/KeyTransport/RSAKeyTransportTest.php @@ -80,12 +80,12 @@ public function testDecrypt(): void "A6fgclGb/keGZOtjSkHZnZEZvXEOQItFjS6MbQc+TzNmRd6FSkuPUmwQ1V+NwxTPCIwXSSd0Aj" . "7oHb7xRdBhoFuDrSbYAvATQ="; $rsa = self::$factory->getAlgorithm(C::KEY_TRANSPORT_OAEP_MGF1P, self::$privateKey); - $plaintext = $rsa->decrypt(base64_decode($ciphertext)); + $plaintext = $rsa->decrypt(base64_decode($ciphertext, true)); $this->assertEquals(self::PLAINTEXT, $plaintext); // test RSA-OAEP (should behave the same as MGF1P) $rsa = self::$factory->getAlgorithm(C::KEY_TRANSPORT_OAEP, self::$privateKey); - $plaintext = $rsa->decrypt(base64_decode($ciphertext)); + $plaintext = $rsa->decrypt(base64_decode($ciphertext, true)); $this->assertEquals(self::PLAINTEXT, $plaintext); // test RSA-1.5 @@ -93,7 +93,7 @@ public function testDecrypt(): void "afiDsy5izSk6+QZ5kMOgRLrmnh+RYZXjvCL6i1NXzaLw8yZLBvlP01SNMv/BBq640yzbG9U2ZN" . "nxBLDvBmbJBxzt6XCowXQS8="; $rsa = self::$factory->getAlgorithm(C::KEY_TRANSPORT_RSA_1_5, self::$privateKey); - $plaintext = $rsa->decrypt(base64_decode($ciphertext)); + $plaintext = $rsa->decrypt(base64_decode($ciphertext, true)); $this->assertEquals(self::PLAINTEXT, $plaintext); } }