Skip to content

[Cocoa] AES-CBC decryption accepts empty ciphertext instead of rejecting with OperationError - #77402

Open
jpowersdev wants to merge 1 commit into
WebKit:mainfrom
jpowersdev:t3/webkit-bug-327956
Open

jpowersdev wants to merge 1 commit into
WebKit:mainfrom
jpowersdev:t3/webkit-bug-327956

Conversation

@jpowersdev

@jpowersdev jpowersdev commented Oct 17, 2026 •

Copy link
Copy Markdown

56a8a1f

[Cocoa] AES-CBC decryption accepts empty ciphertext instead of rejecting with OperationError
https://bugs.webkit.org/show_bug.cgi?id=327956

Reviewed by NOBODY (OOPS!).

The Cocoa backend decrypts without CommonCrypto padding and then validates
PKCS#7 padding itself. It currently skips that validation when the decrypted
data is empty, allowing empty ciphertext to succeed.

Reject empty decrypted data when padding is required, before reading the
padding byte. Preserve unpadded decryption for WebAuthn callers and successful
decryption of a full padding block to empty plaintext.

Add Cocoa API tests for all three cases with 128-, 192-, and 256-bit keys.
These tests allow this fix to land independently of the GCrypt fix and its
layout tests in https://github.com/WebKit/WebKit/pull/77307.

Validation: WebKit style checks and a C++ syntax check of the new API tests
using the GTK developer build pass. Cocoa runtime tests have not been run
locally; they require Apple CI or a macOS developer build.

Tests: TestWebKitAPI.CryptoAlgorithmAESCBC.DecryptEmptyCipherText
       TestWebKitAPI.CryptoAlgorithmAESCBC.EncryptDecryptEmptyPlainText
       TestWebKitAPI.CryptoAlgorithmAESCBC.DecryptEmptyCipherTextWithoutPadding

* Source/WebCore/crypto/cocoa/CryptoAlgorithmAESCBCCocoa.cpp:
(WebCore::CryptoAlgorithmAESCBC::platformDecrypt):
* Tools/TestWebKitAPI/PlatformCocoa.cmake:
* Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* Tools/TestWebKitAPI/Tests/WebCore/cocoa/CryptoAlgorithmAESCBC.cpp: Added.
(TestWebKitAPI::TEST):

56a8a1f

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
loading 🧪 webkitpy ✅ 🧪 ios-wk2-wpt ✅ 🧪 api-mac-debug
✅ 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk3-gcc
✅ 🛠 ios-safer-cpp ✅ 🧪 mac-AS-debug-wk2 ✅ 🛠 gtk
✅ 🛠 vision ✅ 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-intel-wk2 ✅ 🧪 api-gtk
✅ 🧪 vision-wk2 ✅ 🛠 mac-safer-cpp   🛠 playstation
✅ 🛠 tv ✅ 🧪 mac-site-isolation
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

…ing with OperationError

https://bugs.webkit.org/show_bug.cgi?id=327956

Reviewed by NOBODY (OOPS!).

The Cocoa backend decrypts without CommonCrypto padding and then validates
PKCS#7 padding itself. It currently skips that validation when the decrypted
data is empty, allowing empty ciphertext to succeed.

Reject empty decrypted data when padding is required, before reading the
padding byte. Preserve unpadded decryption for WebAuthn callers and successful
decryption of a full padding block to empty plaintext.

Add Cocoa API tests for all three cases with 128-, 192-, and 256-bit keys.
These tests allow this fix to land independently of the GCrypt fix and its
layout tests in WebKit#77307.

Validation: WebKit style checks and a C++ syntax check of the new API tests
using the GTK developer build pass. Cocoa runtime tests have not been run
locally; they require Apple CI or a macOS developer build.

Tests: TestWebKitAPI.CryptoAlgorithmAESCBC.DecryptEmptyCipherText
       TestWebKitAPI.CryptoAlgorithmAESCBC.EncryptDecryptEmptyPlainText
       TestWebKitAPI.CryptoAlgorithmAESCBC.DecryptEmptyCipherTextWithoutPadding

* Source/WebCore/crypto/cocoa/CryptoAlgorithmAESCBCCocoa.cpp:
(WebCore::CryptoAlgorithmAESCBC::platformDecrypt):
* Tools/TestWebKitAPI/PlatformCocoa.cmake:
* Tools/TestWebKitAPI/TestWebKitAPI.xcodeproj/project.pbxproj:
* Tools/TestWebKitAPI/Tests/WebCore/cocoa/CryptoAlgorithmAESCBC.cpp: Added.
(TestWebKitAPI::TEST):
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants