Title: [245911] trunk/Source/WebKit
- Revision
- 245911
- Author
- [email protected]
- Date
- 2019-05-30 17:00:09 -0700 (Thu, 30 May 2019)
Log 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.
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (245910 => 245911)
--- trunk/Source/WebKit/ChangeLog 2019-05-30 23:58:38 UTC (rev 245910)
+++ trunk/Source/WebKit/ChangeLog 2019-05-31 00:00:09 UTC (rev 245911)
@@ -1,3 +1,32 @@
+2019-05-30 Chris Dumez <[email protected]>
+
+ 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-05-30 Sihui Liu <[email protected]>
Stop StorageManager when network process is ready to suspend
Modified: trunk/Source/WebKit/Shared/cf/ArgumentCodersCF.cpp (245910 => 245911)
--- trunk/Source/WebKit/Shared/cf/ArgumentCodersCF.cpp 2019-05-30 23:58:38 UTC (rev 245910)
+++ trunk/Source/WebKit/Shared/cf/ArgumentCodersCF.cpp 2019-05-31 00:00:09 UTC (rev 245911)
@@ -371,7 +371,7 @@
if (!decoder.decode(size))
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) {
RetainPtr<CFTypeRef> element;
@@ -378,6 +378,9 @@
if (!decode(decoder, element))
return false;
+ if (!element)
+ continue;
+
CFArrayAppendValue(array.get(), element.get());
}
Modified: trunk/Source/WebKit/Shared/mac/SecItemResponseData.cpp (245910 => 245911)
--- trunk/Source/WebKit/Shared/mac/SecItemResponseData.cpp 2019-05-30 23:58:38 UTC (rev 245910)
+++ trunk/Source/WebKit/Shared/mac/SecItemResponseData.cpp 2019-05-31 00:00:09 UTC (rev 245911)
@@ -32,8 +32,8 @@
namespace WebKit {
SecItemResponseData::SecItemResponseData(OSStatus resultCode, RetainPtr<CFTypeRef>&& resultObject)
- : m_resultObject(resultObject)
- , m_resultCode(WTFMove(resultCode))
+ : m_resultObject(WTFMove(resultObject))
+ , m_resultCode(resultCode)
{
}
@@ -40,7 +40,7 @@
void SecItemResponseData::encode(IPC::Encoder& encoder) const
{
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());
}
Modified: trunk/Source/WebKit/UIProcess/mac/SecItemShimProxy.cpp (245910 => 245911)
--- trunk/Source/WebKit/UIProcess/mac/SecItemShimProxy.cpp 2019-05-30 23:58:38 UTC (rev 245910)
+++ trunk/Source/WebKit/UIProcess/mac/SecItemShimProxy.cpp 2019-05-31 00:00:09 UTC (rev 245911)
@@ -65,13 +65,13 @@
switch (request.type()) {
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) });
break;
}
@@ -79,19 +79,19 @@
// Return value of SecItemAdd is often ignored. Even if it isn't, we don't have the ability to
// serialize SecKeychainItemRef.
OSStatus resultCode = SecItemAdd(request.query(), nullptr);
- response(SecItemResponseData(resultCode, nullptr));
+ response(SecItemResponseData { resultCode, nullptr });
break;
}
case SecItemRequestData::Update: {
OSStatus resultCode = SecItemUpdate(request.query(), request.attributesToMatch());
- response(SecItemResponseData(resultCode, 0));
+ response(SecItemResponseData { resultCode, nullptr });
break;
}
case SecItemRequestData::Delete: {
OSStatus resultCode = SecItemDelete(request.query());
- response(SecItemResponseData(resultCode, 0));
+ response(SecItemResponseData { resultCode, nullptr });
break;
}
}
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
