Changeset 246648 in webkit

Timestamp:

Jun 20, 2019, 1:06:49 PM (7 years ago)

Author:

Kocsen Chung

Message:

Cherry-pick r245911. rdar://problem/51656609

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.

git-svn-id: ​https://svn.webkit.org/repository/webkit/trunk@245911 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Location:

branches/safari-607-branch/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• Shared/cf/ArgumentCodersCF.cpp (modified) (2 diffs)
• Shared/mac/SecItemResponseData.cpp (modified) (1 diff)
• UIProcess/mac/SecItemShimProxy.cpp (modified) (4 diffs)

branches/safari-607-branch/Source/WebKit/ChangeLog

@@ +1 @@
+2019-06-19  Kocsen Chung  <kocsen_chung@apple.com>
+
+        Cherry-pick r245911. rdar://problem/51656609
+
+    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.
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@245911 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2019-05-30  Chris Dumez  <cdumez@apple.com>
+
+            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.
+
 2019-06-12  Null  <null@apple.com>
 

branches/safari-607-branch/Source/WebKit/Shared/cf/ArgumentCodersCF.cpp

@@ -347 +347 @@
         return false;
 
-    RetainPtr<CFMutableArrayRef> array = adoptCF(CFArrayCreateMutable(0, 0, &kCFTypeArrayCallBacks));
+    auto array = adoptCF(CFArrayCreateMutable(0, 0, &kCFTypeArrayCallBacks));
 
     for (size_t i = 0; i < size; ++i) {
@@ -354 +354 @@
             return false;
 
+        if (!element)
+            continue;
+        
         CFArrayAppendValue(array.get(), element.get());
     }

branches/safari-607-branch/Source/WebKit/Shared/mac/SecItemResponseData.cpp

@@ -45 +45 @@
 {
     encoder << static_cast<int64_t>(m_resultCode);
-    encoder << static_cast<bool>(m_resultObject.get());
+    encoder << static_cast<bool>(m_resultObject);
     if (m_resultObject)
         IPC::encode(encoder, m_resultObject.get());

branches/safari-607-branch/Source/WebKit/UIProcess/mac/SecItemShimProxy.cpp

@@ -66 +66 @@
     case SecItemRequestData::Invalid:
         LOG_ERROR("SecItemShimProxy::secItemRequest received an invalid data request. Please file a bug if you know how you caused this.");
-        response = SecItemResponseData(errSecParam, nullptr);
+        response = SecItemResponseData { errSecParam, nullptr };
         break;
 
     case SecItemRequestData::CopyMatching: {
-        CFTypeRef resultObject = 0;
+        CFTypeRef resultObject = nullptr;
         OSStatus resultCode = SecItemCopyMatching(request.query(), &resultObject);
-        response = SecItemResponseData(resultCode, adoptCF(resultObject).get());
+        response = SecItemResponseData { resultCode, adoptCF(resultObject).get() };
         break;
     }
@@ -80 +80 @@
         // serialize SecKeychainItemRef.
         OSStatus resultCode = SecItemAdd(request.query(), nullptr);
-        response = SecItemResponseData(resultCode, nullptr);
+        response = SecItemResponseData { resultCode, nullptr };
         break;
     }
@@ -86 +86 @@
     case SecItemRequestData::Update: {
         OSStatus resultCode = SecItemUpdate(request.query(), request.attributesToMatch());
-        response = SecItemResponseData(resultCode, 0);
+        response = SecItemResponseData { resultCode, nullptr };
         break;
     }
@@ -92 +92 @@
     case SecItemRequestData::Delete: {
         OSStatus resultCode = SecItemDelete(request.query());
-        response = SecItemResponseData(resultCode, 0);
+        response = SecItemResponseData { resultCode, nullptr };
         break;
     }