Bug 222084 - Crash in IPC::decode(Decoder& decoder, RetainPtr<SecKeychainItemRef>& result)
Summary: Crash in IPC::decode(Decoder& decoder, RetainPtr<SecKeychainItemRef>& result)
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified macOS 11
: P2 Normal
Assignee: Nobody
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-02-17 17:51 PST by Julian Gonzalez
Modified: 2021-02-19 11:50 PST (History)
4 users (show)

See Also:


Attachments
Patch (4.15 KB, patch)
2021-02-17 18:04 PST, Julian Gonzalez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Gonzalez 2021-02-17 17:51:52 PST
e.g.

    #1 0x7fff223e01bc in SecKeychainItemCopyFromPersistentReference+0x176
    #2 0x118cfa27b in IPC::decode(IPC::Decoder&, WTF::RetainPtr<__SecKeychainItem*>&) ArgumentCodersCF.cpp:672
    #3 0x118cf6078 in IPC::decode(IPC::Decoder&, WTF::RetainPtr<void const*>&) ArgumentCodersCF.cpp:259
    #4 0x118cf8228 in IPC::decode(IPC::Decoder&, WTF::RetainPtr<__CFDictionary const*>&) ArgumentCodersCF.cpp:461
    #5 0x11a0056d7 in IPC::decodeNSError(IPC::Decoder&, WTF::RetainPtr<NSError>&) WebCoreArgumentCodersMac.mm:411
    #6 0x11a005406 in IPC::ArgumentCoder<WebCore::ResourceError>::decodePlatformData(IPC::Decoder&, WebCore::ResourceError&) WebCoreArgumentCodersMac.mm:435

SecKeychainItemCopyFromPersistentReference() cannot handle CFDataRefs with length 0.

<rdar://problem/73568808>
Comment 1 Julian Gonzalez 2021-02-17 18:04:01 PST
Created attachment 420776 [details]
Patch
Comment 2 Julian Gonzalez 2021-02-18 15:44:48 PST
Failing Windows tests here do not seem to be related - this is a change in Mac only code.
Comment 3 Ryosuke Niwa 2021-02-18 17:13:20 PST
Yeah http/tests/xmlhttprequest/redirect-cross-origin-sync-double.html isn't always failing.
Comment 4 EWS 2021-02-18 17:16:33 PST
Committed r273116: <https://commits.webkit.org/r273116>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 420776 [details].
Comment 5 Darin Adler 2021-02-18 17:29:15 PST
Comment on attachment 420776 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=420776&action=review

> Source/WebKit/Shared/cf/ArgumentCodersCF.cpp:671
> +    CFDataRef dref = data.get();

For future reference, in a case like this there is no need for the local variable. It’s totally fine to call data.get() twice.
Comment 6 Ryosuke Niwa 2021-02-19 11:50:03 PST
(In reply to Darin Adler from comment #5)
> Comment on attachment 420776 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=420776&action=review
> 
> > Source/WebKit/Shared/cf/ArgumentCodersCF.cpp:671
> > +    CFDataRef dref = data.get();
> 
> For future reference, in a case like this there is no need for the local
> variable. It’s totally fine to call data.get() twice.

That's a good point. The compiler will do the work for us since get() is a simple accessor.