fix: normalize SodiumHandler params and padding failures

system/Encryption/Handlers/SodiumHandler.php

@@ -43,36 +43,32 @@ class SodiumHandler extends BaseHandler
      */
     public function encrypt(#[SensitiveParameter] $data, #[SensitiveParameter] $params = null)
     {
-        // Allow key override
-        $key = $params !== null
-            ? (is_array($params) && isset($params['key']) ? $params['key'] : $params)
-            : $this->key;
-
-        // Allow blockSize override
-        $blockSize = (is_array($params) && isset($params['blockSize']))
-            ? $params['blockSize']
-            : $this->blockSize;
+        $key       = $this->key;
+        $blockSize = $this->blockSize;
+
+        if ($params !== null) {
+            if (is_array($params)) {
+                $key       = array_key_exists('key', $params) ? $params['key'] : $key;
+                $blockSize = array_key_exists('blockSize', $params) ? $params['blockSize'] : $blockSize;
+            } else {
+                $key = $params;
+            }
+        }
 
-        if (empty($key)) {
+        if (empty($key) || strlen((string) $key) !== SODIUM_CRYPTO_SECRETBOX_KEYBYTES) {
             throw EncryptionException::forNeedsStarterKey();
         }
 
-        // create a nonce for this operation
-        $nonce = random_bytes(SODIUM_CRYPTO_SECRETBOX_NONCEBYTES); // 24 bytes
-
-        // add padding before we encrypt the data
         if ($blockSize <= 0) {
             throw EncryptionException::forEncryptionFailed();
         }
 
-        $data = sodium_pad($data, $blockSize);
+        $nonce = random_bytes(SODIUM_CRYPTO_SECRETBOX_NONCEBYTES);
+        $data  = sodium_pad($data, $blockSize);
 
-        // encrypt message and combine with nonce
         $ciphertext = $nonce . sodium_crypto_secretbox($data, $nonce, $key);
 
-        // cleanup buffers
         sodium_memzero($data);
-        sodium_memzero($key);
 
         return $ciphertext;
     }
@@ -82,47 +78,38 @@ public function encrypt(#[SensitiveParameter] $data, #[SensitiveParameter] $para
      */
     public function decrypt($data, #[SensitiveParameter] $params = null)
     {
-        // Allow key override
-        $key = $params !== null
-            ? (is_array($params) && isset($params['key']) ? $params['key'] : $params)
-            : $this->key;
-
-        // Allow blockSize override
-        $blockSize = (is_array($params) && isset($params['blockSize']))
-            ? $params['blockSize']
-            : $this->blockSize;
+        $key       = $this->key;
+        $blockSize = $this->blockSize;
+
+        if ($params !== null) {
+            if (is_array($params)) {
+                $key       = array_key_exists('key', $params) ? $params['key'] : $key;
+                $blockSize = array_key_exists('blockSize', $params) ? $params['blockSize'] : $blockSize;
+            } else {
+                $key = $params;
+            }
+        }
 
-        if (empty($key)) {
+        if (empty($key) || strlen((string) $key) !== SODIUM_CRYPTO_SECRETBOX_KEYBYTES) {
             throw EncryptionException::forNeedsStarterKey();
         }
 
         if (mb_strlen($data, '8bit') < (SODIUM_CRYPTO_SECRETBOX_NONCEBYTES + SODIUM_CRYPTO_SECRETBOX_MACBYTES)) {
-            // message was truncated
             throw EncryptionException::forAuthenticationFailed();
         }
 
-        // Extract info from encrypted data
         $nonce      = self::substr($data, 0, SODIUM_CRYPTO_SECRETBOX_NONCEBYTES);
         $ciphertext = self::substr($data, SODIUM_CRYPTO_SECRETBOX_NONCEBYTES);
 
-        // decrypt data
         $data = sodium_crypto_secretbox_open($ciphertext, $nonce, $key);
 
-        if ($data === false) {
-            // message was tampered in transit
-            throw EncryptionException::forAuthenticationFailed(); // @codeCoverageIgnore
-        }
-
-        // remove extra padding during encryption
-        if ($blockSize <= 0) {
+        if ($data === false || $blockSize <= 0) {
             throw EncryptionException::forAuthenticationFailed();
         }
 
         $data = sodium_unpad($data, $blockSize);
 
-        // cleanup buffers
         sodium_memzero($ciphertext);
-        sodium_memzero($key);
 
         return $data;
     }

tests/system/Encryption/Handlers/SodiumHandlerTest.php

@@ -18,6 +18,7 @@
 use CodeIgniter\Test\CIUnitTestCase;
 use Config\Encryption as EncryptionConfig;
 use PHPUnit\Framework\Attributes\Group;
+use SodiumException;
 
 /**
  * @internal
@@ -151,4 +152,78 @@ public function testInternalKeyNotModifiedByParams(): void
 
         $this->assertSame($message, $encrypter->decrypt($encoded, ['key' => $differentKey]));
     }
+
+    public function testBug0IssetThrowsTypeError(): void
+    {
+        $this->expectException(EncryptionException::class);
+        /** @var SodiumHandler $encrypter */
+        $encrypter = $this->encryption->initialize($this->config);
+
+        $encrypter->encrypt('message', ['key' => null]);
+    }
+
+    public function testBug1MemzeroZeroesInternalKey(): void
+    {
+        $encrypter = $this->encryption->initialize($this->config);
+
+        $prop        = 'key';
+        $originalKey = $encrypter->{$prop}; // @phpstan-ignore property.notFound
+
+        $encrypter->encrypt('message');
+
+        $this->assertSame($originalKey, $encrypter->{$prop}); // @phpstan-ignore property.notFound
+    }
+
+    public function testBug2InvalidKeyLengthThrowsSodiumException(): void
+    {
+        $this->expectException(EncryptionException::class);
+        /** @var SodiumHandler $encrypter */
+        $encrypter = $this->encryption->initialize($this->config);
+
+        $encrypter->encrypt('message', str_repeat('a', 31));
+    }
+
+    public function testBug3InvalidPaddingThrowsSodiumException(): void
+    {
+        $this->expectException(SodiumException::class);
+        /** @var SodiumHandler $encrypter */
+        $encrypter = $this->encryption->initialize($this->config);
+
+        $ciphertext = $encrypter->encrypt('message', ['blockSize' => 16]);
+
+        $encrypter->decrypt($ciphertext, ['blockSize' => 32]);
+    }
+
+    public function testBug4OriginalDataIsNotZeroed(): void
+    {
+        $encrypter = $this->encryption->initialize($this->config);
+
+        $message = 'SuperSecretMessage';
+        $encrypter->encrypt($message);
+
+        $this->assertSame('SuperSecretMessage', $message);
+    }
+
+    public function testDecryptTamperedMessageThrowsException(): void
+    {
+        $this->expectException(EncryptionException::class);
+        $encrypter = $this->encryption->initialize($this->config);
+
+        $ciphertext = $encrypter->encrypt('message');
+
+        $ciphertext[0] = $ciphertext[0] === 'a' ? 'b' : 'a';
+
+        $encrypter->decrypt($ciphertext);
+    }
+
+    public function testOverrideKeyAsStringWorks(): void
+    {
+        $encrypter = $this->encryption->initialize($this->config);
+        $newKey    = sodium_crypto_secretbox_keygen();
+
+        $ciphertext = $encrypter->encrypt('message', $newKey);
+        $decrypted  = $encrypter->decrypt($ciphertext, $newKey);
+
+        $this->assertSame('message', $decrypted);
+    }
 }