Changeset 245911 in webkit


Ignore:
Timestamp:
May 30, 2019 5:00:09 PM (5 years ago)
Author:
Chris Dumez
Message:

Network process crash when decoding SecItemResponseData
https://bugs.webkit.org/show_bug.cgi?id=198388
<rdar://problem/50408046>

Reviewed by Alex Christensen.

  • Shared/cf/ArgumentCodersCF.cpp:

(IPC::decode):
When decoding the elements inside a CFArrayRef, if decoding was successful but
the CFTypeRef element is still null then skip it instead of trying to append it
to the array. A CFArray container is not allowed to contain null.
Some of our decoders for CFTypeRef types may not initialize the element even if
the decode() function returns true. For example, the decoders for CFArrayRef and
CFDictionaryRef return true if the encoded container was null but do not create
a container.

  • Shared/mac/SecItemResponseData.cpp:

(WebKit::SecItemResponseData::SecItemResponseData):
nit: The wrong parameter was being moved. This is more efficient.

(WebKit::SecItemResponseData::encode const):
nit: Drop unnecessary .get().

  • UIProcess/mac/SecItemShimProxy.cpp:

(WebKit::SecItemShimProxy::secItemRequest):
nit: Use nullptr instead of 0.

Location:
trunk/Source/WebKit
Files:
4 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebKit/ChangeLog

    r245904 r245911  
     12019-05-30  Chris Dumez  <cdumez@apple.com>
     2
     3        Network process crash when decoding SecItemResponseData
     4        https://bugs.webkit.org/show_bug.cgi?id=198388
     5        <rdar://problem/50408046>
     6
     7        Reviewed by Alex Christensen.
     8
     9        * Shared/cf/ArgumentCodersCF.cpp:
     10        (IPC::decode):
     11        When decoding the elements inside a CFArrayRef, if decoding was successful but
     12        the CFTypeRef element is still null then skip it instead of trying to append it
     13        to the array. A CFArray container is not allowed to contain null.
     14        Some of our decoders for CFTypeRef types may not initialize the element even if
     15        the decode() function returns true. For example, the decoders for CFArrayRef and
     16        CFDictionaryRef return true if the encoded container was null but do not create
     17        a container.
     18
     19        * Shared/mac/SecItemResponseData.cpp:
     20        (WebKit::SecItemResponseData::SecItemResponseData):
     21        nit: The wrong parameter was being moved. This is more efficient.
     22
     23        (WebKit::SecItemResponseData::encode const):
     24        nit: Drop unnecessary .get().
     25
     26        * UIProcess/mac/SecItemShimProxy.cpp:
     27        (WebKit::SecItemShimProxy::secItemRequest):
     28        nit: Use nullptr instead of 0.
     29
    1302019-05-30  Sihui Liu  <sihui_liu@apple.com>
    231
  • trunk/Source/WebKit/Shared/cf/ArgumentCodersCF.cpp

    r245468 r245911  
    372372        return false;
    373373
    374     RetainPtr<CFMutableArrayRef> array = adoptCF(CFArrayCreateMutable(0, 0, &kCFTypeArrayCallBacks));
     374    auto array = adoptCF(CFArrayCreateMutable(0, 0, &kCFTypeArrayCallBacks));
    375375
    376376    for (size_t i = 0; i < size; ++i) {
     
    379379            return false;
    380380
     381        if (!element)
     382            continue;
     383       
    381384        CFArrayAppendValue(array.get(), element.get());
    382385    }
  • trunk/Source/WebKit/Shared/mac/SecItemResponseData.cpp

    r243460 r245911  
    3333
    3434SecItemResponseData::SecItemResponseData(OSStatus resultCode, RetainPtr<CFTypeRef>&& resultObject)
    35     : m_resultObject(resultObject)
    36     , m_resultCode(WTFMove(resultCode))
     35    : m_resultObject(WTFMove(resultObject))
     36    , m_resultCode(resultCode)
    3737{
    3838}
     
    4141{
    4242    encoder << static_cast<int64_t>(m_resultCode);
    43     encoder << static_cast<bool>(m_resultObject.get());
     43    encoder << static_cast<bool>(m_resultObject);
    4444    if (m_resultObject)
    4545        IPC::encode(encoder, m_resultObject.get());
  • trunk/Source/WebKit/UIProcess/mac/SecItemShimProxy.cpp

    r241441 r245911  
    6666    case SecItemRequestData::Invalid:
    6767        LOG_ERROR("SecItemShimProxy::secItemRequest received an invalid data request. Please file a bug if you know how you caused this.");
    68         response(SecItemResponseData(errSecParam, nullptr));
     68        response(SecItemResponseData { errSecParam, nullptr });
    6969        break;
    7070
    7171    case SecItemRequestData::CopyMatching: {
    72         CFTypeRef resultObject = 0;
     72        CFTypeRef resultObject = nullptr;
    7373        OSStatus resultCode = SecItemCopyMatching(request.query(), &resultObject);
    74         response(SecItemResponseData(resultCode, adoptCF(resultObject).get()));
     74        response(SecItemResponseData { resultCode, adoptCF(resultObject) });
    7575        break;
    7676    }
     
    8080        // serialize SecKeychainItemRef.
    8181        OSStatus resultCode = SecItemAdd(request.query(), nullptr);
    82         response(SecItemResponseData(resultCode, nullptr));
     82        response(SecItemResponseData { resultCode, nullptr });
    8383        break;
    8484    }
     
    8686    case SecItemRequestData::Update: {
    8787        OSStatus resultCode = SecItemUpdate(request.query(), request.attributesToMatch());
    88         response(SecItemResponseData(resultCode, 0));
     88        response(SecItemResponseData { resultCode, nullptr });
    8989        break;
    9090    }
     
    9292    case SecItemRequestData::Delete: {
    9393        OSStatus resultCode = SecItemDelete(request.query());
    94         response(SecItemResponseData(resultCode, 0));
     94        response(SecItemResponseData { resultCode, nullptr });
    9595        break;
    9696    }
Note: See TracChangeset for help on using the changeset viewer.