Title: [87627] trunk/Source/WebKit2
- Revision
- 87627
- Author
- [email protected]
- Date
- 2011-05-28 16:45:17 -0700 (Sat, 28 May 2011)
Log Message
2011-05-28 Simon Fraser <[email protected]>
Reviewed by Dan Bernstein, Maciej Stachowiak.
Denying access to your keychain on login crashes WebKit2
https://bugs.webkit.org/show_bug.cgi?id=61695
<rdar://problem/9520570>
Fix two sources of crashes if you hit the Deny button when WebKit2 is
doing HTTP authentication.
First, SecKeychainItemRequestData::attributeList() failed to initialize the
length and data members of SecKeychainAttributes in the list if there was no data.
This caused invalid memory reads later.
Second, returning a non-zero error from the SecKeychainItemCopyContent shim method
would cause a later crash in a system framework, which is not set up to handle
errors. Instead, we always return noErr, and allow the authentication to fail.
Finally, paranoically initialize the SecKeychainItemContext in two places
to avoid uninitialized data members, and initialize length and outData
to 0 in secKeychainItemCopyContent() in case SecKeychainItemCopyContent()
fails to set them on error.
* Shared/mac/SecKeychainItemRequestData.cpp:
(WebKit::SecKeychainItemRequestData::attributeList):
* UIProcess/mac/WebProcessProxyMac.mm:
(WebKit::WebProcessProxy::secKeychainItemCopyContent):
* WebProcess/mac/KeychainItemShimMethods.mm:
(WebKit::webSecKeychainItemCopyContent):
(WebKit::webSecKeychainItemCreateFromContent):
Modified Paths
Diff
Modified: trunk/Source/WebKit2/ChangeLog (87626 => 87627)
--- trunk/Source/WebKit2/ChangeLog 2011-05-28 22:26:15 UTC (rev 87626)
+++ trunk/Source/WebKit2/ChangeLog 2011-05-28 23:45:17 UTC (rev 87627)
@@ -1,3 +1,35 @@
+2011-05-28 Simon Fraser <[email protected]>
+
+ Reviewed by Dan Bernstein, Maciej Stachowiak.
+
+ Denying access to your keychain on login crashes WebKit2
+ https://bugs.webkit.org/show_bug.cgi?id=61695
+ <rdar://problem/9520570>
+
+ Fix two sources of crashes if you hit the Deny button when WebKit2 is
+ doing HTTP authentication.
+
+ First, SecKeychainItemRequestData::attributeList() failed to initialize the
+ length and data members of SecKeychainAttributes in the list if there was no data.
+ This caused invalid memory reads later.
+
+ Second, returning a non-zero error from the SecKeychainItemCopyContent shim method
+ would cause a later crash in a system framework, which is not set up to handle
+ errors. Instead, we always return noErr, and allow the authentication to fail.
+
+ Finally, paranoically initialize the SecKeychainItemContext in two places
+ to avoid uninitialized data members, and initialize length and outData
+ to 0 in secKeychainItemCopyContent() in case SecKeychainItemCopyContent()
+ fails to set them on error.
+
+ * Shared/mac/SecKeychainItemRequestData.cpp:
+ (WebKit::SecKeychainItemRequestData::attributeList):
+ * UIProcess/mac/WebProcessProxyMac.mm:
+ (WebKit::WebProcessProxy::secKeychainItemCopyContent):
+ * WebProcess/mac/KeychainItemShimMethods.mm:
+ (WebKit::webSecKeychainItemCopyContent):
+ (WebKit::webSecKeychainItemCreateFromContent):
+
2011-05-28 Anders Carlsson <[email protected]>
Reviewed by Dan Bernstein.
Modified: trunk/Source/WebKit2/Shared/mac/SecKeychainItemRequestData.cpp (87626 => 87627)
--- trunk/Source/WebKit2/Shared/mac/SecKeychainItemRequestData.cpp 2011-05-28 22:26:15 UTC (rev 87626)
+++ trunk/Source/WebKit2/Shared/mac/SecKeychainItemRequestData.cpp 2011-05-28 23:45:17 UTC (rev 87627)
@@ -101,8 +101,11 @@
for (size_t i = 0; i < m_attributeList->count; ++i) {
m_attributeList->attr[i].tag = m_keychainAttributes[i].tag;
- if (!m_keychainAttributes[i].data)
+ if (!m_keychainAttributes[i].data) {
+ m_attributeList->attr[i].length = 0;
+ m_attributeList->attr[i].data = ""
continue;
+ }
m_attributeList->attr[i].length = CFDataGetLength(m_keychainAttributes[i].data.get());
m_attributeList->attr[i].data = "" void*>(CFDataGetBytePtr(m_keychainAttributes[i].data.get())));
Modified: trunk/Source/WebKit2/UIProcess/mac/WebProcessProxyMac.mm (87626 => 87627)
--- trunk/Source/WebKit2/UIProcess/mac/WebProcessProxyMac.mm 2011-05-28 22:26:15 UTC (rev 87626)
+++ trunk/Source/WebKit2/UIProcess/mac/WebProcessProxyMac.mm 2011-05-28 23:45:17 UTC (rev 87627)
@@ -82,8 +82,8 @@
SecKeychainItemRef item = request.keychainItem();
SecItemClass itemClass;
SecKeychainAttributeList* attrList = request.attributeList();
- UInt32 length;
- void* outData;
+ UInt32 length = 0;
+ void* outData = 0;
OSStatus resultCode = SecKeychainItemCopyContent(item, &itemClass, attrList, &length, &outData);
Modified: trunk/Source/WebKit2/WebProcess/mac/KeychainItemShimMethods.mm (87626 => 87627)
--- trunk/Source/WebKit2/WebProcess/mac/KeychainItemShimMethods.mm 2011-05-28 22:26:15 UTC (rev 87626)
+++ trunk/Source/WebKit2/WebProcess/mac/KeychainItemShimMethods.mm 2011-05-28 23:45:17 UTC (rev 87627)
@@ -208,6 +208,7 @@
static OSStatus webSecKeychainItemCopyContent(SecKeychainItemRef item, SecItemClass* itemClass, SecKeychainAttributeList* attrList, UInt32* length, void** outData)
{
SecKeychainItemContext context;
+ memset(&context, 0, sizeof(SecKeychainItemContext));
context.item = item;
context.resultItemClass = itemClass;
context.attributeList = attrList;
@@ -216,7 +217,9 @@
callOnMainThreadAndWait(webSecKeychainItemCopyContentOnMainThread, &context);
- return context.resultCode;
+ // FIXME: should return context.resultCode. Returning noErr is a workaround for <rdar://problem/9520886>;
+ // the authentication should fail anyway, since on error no data will be returned.
+ return noErr;
}
static void webSecKeychainItemCreateFromContentOnMainThread(void* voidContext)
@@ -239,6 +242,7 @@
static OSStatus webSecKeychainItemCreateFromContent(SecItemClass itemClass, SecKeychainAttributeList* attrList, UInt32 length, const void* data, SecKeychainItemRef *item)
{
SecKeychainItemContext context;
+ memset(&context, 0, sizeof(SecKeychainItemContext));
context.initialItemClass = itemClass;
context.attributeList = attrList;
context.length = length;
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
