Bug 211777

Summary: SubtleCrypto.decrypt() - Decrypting with wrong AES-CBC key succeeds instead throwing an error
Product: WebKit Reporter: Pavel Bednar <pavel.bednar>
Component: JavaScriptCoreAssignee: Nobody <webkit-unassigned>
Status: RESOLVED INVALID    
Severity: Critical CC: jiewen_tan, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: Safari 13   
Hardware: Mac   
OS: macOS 10.15   
Attachments:
Description Flags
repro sample none

Description Pavel Bednar 2020-05-12 04:38:46 PDT
Created attachment 399118 [details]
repro sample

We are developing a bussiness app utilizing Web Crypto API.  When user tryies to decrypt data with wrong key, SubtleCrypto.decrypt() should throw an error regarding to https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto/decrypt.
In Safari instead of an error it succeed and returns some mismatched data. This is serious bug since we do not have any means to detect unsuccessfull decryption
Same test case in Chrome throws an error, also using another algorithm e.g. AES-CGM in safari also throw errors. It also worked in previous versions of Safari (we tested this several months ago and was throwing error). This bug seems to be related only to AES-CBS.

Bellow I attached simple repro in javascript.
Comment 1 Radar WebKit Bug Importer 2020-05-12 13:55:30 PDT
<rdar://problem/63152520>
Comment 2 Jiewen Tan 2020-05-12 15:57:06 PDT
That's the design of the AES-CBC. Decryptions are designed to fail silently. Otherwise, attackers don't even need to examine the content the decrypted message to determine if the brute force attack succeeds or not. For integration protection, either adding HMAC to your cipher or using AES-GCM.
Comment 3 Pavel Bednar 2020-05-12 22:35:03 PDT
I cannot agree, few months it was working as expected. I dont see why decrypt method should behave inconsistently based on selected algorithm. AES-GCM in same repro throws an error. All major browsers throws an error (tested in Chrome, Firefox, Opera, Edge). Documentation says it should throw an error. Only webkit started to fail silently but just only for this particular algorithm. 

Unfortunately we are not able to upgrade AES-GCM or simply add HMAC since our custumers already have encrypted data in production and we have to maintain compatibility.
Comment 4 Jiewen Tan 2020-05-13 11:38:26 PDT
(In reply to Pavel Bednar from comment #3)
> I cannot agree, few months it was working as expected. I dont see why
> decrypt method should behave inconsistently based on selected algorithm.
> AES-GCM in same repro throws an error. All major browsers throws an error
> (tested in Chrome, Firefox, Opera, Edge). Documentation says it should throw
> an error. Only webkit started to fail silently but just only for this
> particular algorithm. 
> 
> Unfortunately we are not able to upgrade AES-GCM or simply add HMAC since
> our custumers already have encrypted data in production and we have to
> maintain compatibility.

If you think this is a regression, then please provide information on when it works and when it starts failing.
Comment 5 Jiewen Tan 2020-05-13 11:43:55 PDT
(In reply to Pavel Bednar from comment #3)
> I cannot agree, few months it was working as expected. I dont see why
> decrypt method should behave inconsistently based on selected algorithm.
> AES-GCM in same repro throws an error. All major browsers throws an error
> (tested in Chrome, Firefox, Opera, Edge). Documentation says it should throw
> an error. Only webkit started to fail silently but just only for this
> particular algorithm. 
> 
> Unfortunately we are not able to upgrade AES-GCM or simply add HMAC since
> our custumers already have encrypted data in production and we have to
> maintain compatibility.

BTW, MDM is not the spec. This is: https://www.w3.org/TR/WebCryptoAPI/#aes-cbc-operations. I don't think it suggests anything about throwing an error when decryptions fail.
Comment 6 Pavel Bednar 2020-05-14 05:06:00 PDT
Ok, can you please explain me, why decrypt() behaves differently for AES-GCM ? Why this algorithm throws an error. Is is not the same vulnerability?
Comment 7 Jiewen Tan 2020-05-14 11:27:15 PDT
(In reply to Pavel Bednar from comment #6)
> Ok, can you please explain me, why decrypt() behaves differently for AES-GCM
> ? Why this algorithm throws an error. Is is not the same vulnerability?

AES-GCM throws exceptions because of integrity not decryption. It first checks the integrity of the cipher. If it fails, then it throws error. Once this point is passed, it behaves more or less the same as AES-CBC.