Bug 129193

Summary: CryptoAlgorithmRSASSA_PKCS1_v1_5::platformVerify contains seemingly accidental unreachable code
Product: WebKit Reporter: Joseph Pecoraro <joepeck>
Component: WebCore Misc.Assignee: Alexey Proskuryakov <ap>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, commit-queue, joepeck, sam
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
proposed fix none

Joseph Pecoraro
Reported 2014-02-21 20:39:19 PST
Compiling CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp with -Wunreachable-code catches: Source/WebCore/crypto/mac/CryptoAlgorithmRSASSA_PKCS1_v1_5Mac.cpp:94:24: error: will never be executed [-Werror,-Wunreachable-code] It looks like a legit warning: void CryptoAlgorithmRSASSA_PKCS1_v1_5::platformVerify(const CryptoAlgorithmRsaSsaParams& parameters, const CryptoKeyRSA& key, const CryptoOperationData& signature, const CryptoOperationData& data, BoolCallback callback, VoidCallback failureCallback, ExceptionCode& ec) { ... CCCryptorStatus status = CCRSACryptorVerify(key.platformKey(), ccPKCS1Padding, digestData.data(), digestData.size(), digestAlgorithm, 0, signature.first, signature.second); if (!status) callback(true); else if (status == kCCNotVerified || kCCDecodeError) // <rdar://problem/15464982> CCRSACryptorVerify returns kCCDecodeError instead of kCCNotVerified sometimes callback(false); else failureCallback(); } --- The "|| kCCDecodeError" part looks suspicious. Perhaps that should be comparing with status, otherwise it looks like it will always be true, and the "else failureCallback()" will never be reached.
Attachments
proposed fix (1.69 KB, patch)
2014-02-21 20:51 PST, Alexey Proskuryakov
no flags
Alexey Proskuryakov
Comment 1 2014-02-21 20:51:01 PST
Created attachment 224943 [details] proposed fix
Joseph Pecoraro
Comment 2 2014-02-21 21:20:03 PST
Comment on attachment 224943 [details] proposed fix r=me! Should we write a test?
Alexey Proskuryakov
Comment 3 2014-02-21 21:30:14 PST
Comment on attachment 224943 [details] proposed fix > Should we write a test? I don't think that there is any way to get to the failureCallback() branch in practice.
WebKit Commit Bot
Comment 4 2014-02-21 22:02:26 PST
Comment on attachment 224943 [details] proposed fix Clearing flags on attachment: 224943 Committed r164528: <http://trac.webkit.org/changeset/164528>
WebKit Commit Bot
Comment 5 2014-02-21 22:02:33 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.