Bug 222084 – Crash in IPC::decode(Decoder& decoder, RetainPtr<SecKeychainItemRef>& result)

• Component: WebKit2
• Status: RESOLVED FIXED

[Julian Gonzalez]

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>

[Julian Gonzalez]

Created attachment 420776 [details]
Patch

[Julian Gonzalez]

Failing Windows tests here do not seem to be related - this is a change in Mac only code.

[Ryosuke Niwa]

Yeah http/tests/xmlhttprequest/redirect-cross-origin-sync-double.html isn't always failing.

[EWS]

Committed r273116: <https://commits.webkit.org/r273116>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 420776 [details].

[Darin Adler]

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.

[Ryosuke Niwa]

(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.