fix: normalize SodiumHandler params and padding failures#10321
Conversation
michalsn
left a comment
There was a problem hiding this comment.
Please rename the new testBug* methods to behavior-focused names. For example, testBug2InvalidKeyLengthThrowsSodiumException() expects EncryptionException, so the current name is misleading.
I also think testBug1MemzeroZeroesInternalKey() and testBug4OriginalDataIsNotZeroed() are questionable as regression tests. Can we either remove them or rewrite them around the actual behavior we want to protect?
Co-authored-by: Michal Sniatala <michal@sniatala.pl>
michalsn
left a comment
There was a problem hiding this comment.
Looks good, thank you. Please add a changelog entry.
For future PRs, please use the PR template when writing the description.
8fd295e to
4c47247
Compare
|
For future, when updating your PR, please use rebase instead of merge: The |
| - **Commands:** Fixed a bug where ``spark lang:find`` treated translation keys already provided by the framework or another namespace (such as ``Errors.*`` in ``system/Language``) as new, listing them under ``--show-new`` and writing untranslated placeholders into ``app/Language`` that overrode the existing translations. | ||
| - **Config:** Fixed a bug where ``BaseService::injectMock`` did not apply ``strtolower`` consistently, causing inconsistent Service and Mock registration and resolution. | ||
| - **Database:** Fixed a bug where ``updateBatch()`` could be called after Query Builder ``where()`` conditions, even though it's not supported. In this situation, now the ``DatabaseException`` is thrown. | ||
| - **Encryption:** Fixed bugs in ``SodiumHandler`` where incorrect key handling and mismatched block sizes caused encryption and decryption failures. |
There was a problem hiding this comment.
| - **Encryption:** Fixed bugs in ``SodiumHandler`` where incorrect key handling and mismatched block sizes caused encryption and decryption failures. | |
| - **Encryption:** Fixed bugs in ``SodiumHandler`` where runtime ``blockSize`` overrides without ``key`` were handled incorrectly, invalid key lengths could leak native Sodium errors, and mismatched decrypt ``blockSize`` values could throw ``SodiumException`` instead of ``EncryptionException``. |
This PR fixes bugs and improves the validation and error handling logic of the encrypt and decrypt methods in the
SodiumHandler.Key changes:
blockSizecan now be passed without also passingkey, while['key' => null]falls back to the configured instance key.SODIUM_CRYPTO_SECRETBOX_KEYBYTES) to ensure invalid Sodium keys throwEncryptionExceptioninstead of native Sodium errors.sodium_unpad()failures so mismatchedblockSizeduring decrypt throwsEncryptionException.$keyand$blockSizefrom the$paramsarray.