<rdar://problem/8960434> and https://bugs.webkit.org/show_bug.cgi?id=53957
authorbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 8 Feb 2011 00:22:47 +0000 (00:22 +0000)
committerbeidson@apple.com <beidson@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 8 Feb 2011 00:22:47 +0000 (00:22 +0000)
Crash after incorrectly restoring bogus session state.

Reviewed by Anders Carlsson.

In some cases we're writing an invalid session state for a back/forward list where the current entry is 0
but the number of entries is also 0.
In such cases the current entry should be "NoCurrentEntryIndex."

When we later read this state in, we set ourselves up to crash later.

Amusingly an ASSERT caught this, but we should've rejected it before the ASSERT fired.

* UIProcess/cf/WebBackForwardListCF.cpp:
(WebKit::WebBackForwardList::restoreFromCFDictionaryRepresentation): Fail the restore if the "current index past the end
  of the list" case occurs, and speculatively bail out of the case where we have no current index but do have a list.
  Also remove the unhelpful ASSERT.

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@77861 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/cf/WebBackForwardListCF.cpp

index 11ba7366eb72e4571fa7c754f61a9f908fcd43dd..cbbcdd8bb0a73086933b7412b1442ed96c97861e 100644 (file)
@@ -1,3 +1,23 @@
+2011-02-07  Brady Eidson  <beidson@apple.com>
+
+        Reviewed by Anders Carlsson.
+
+        <rdar://problem/8960434> and https://bugs.webkit.org/show_bug.cgi?id=53957
+        Crash after incorrectly restoring bogus session state.
+
+        In some cases we're writing an invalid session state for a back/forward list where the current entry is 0
+        but the number of entries is also 0.
+        In such cases the current entry should be "NoCurrentEntryIndex."
+
+        When we later read this state in, we set ourselves up to crash later.
+
+        Amusingly an ASSERT caught this, but we should've rejected it before the ASSERT fired.
+
+        * UIProcess/cf/WebBackForwardListCF.cpp:
+        (WebKit::WebBackForwardList::restoreFromCFDictionaryRepresentation): Fail the restore if the "current index past the end
+          of the list" case occurs, and speculatively bail out of the case where we have no current index but do have a list.
+          Also remove the unhelpful ASSERT.
+
 2011-02-07  Sam Weinig  <sam@webkit.org>
 
         Reviewed by Anders Carlsson.
index cafa3103b98837ffe599808b873c31188796f9cd..6102283fd4b44f5ba4ea342141802d38065dd2d7 100644 (file)
@@ -103,6 +103,16 @@ bool WebBackForwardList::restoreFromCFDictionaryRepresentation(CFDictionaryRef d
     }
 
     CFIndex size = CFArrayGetCount(cfEntries);
+    if (currentIndex != NoCurrentItemIndex && currentIndex >= size) {
+        LOG(SessionState, "WebBackForwardList dictionary representation contains an invalid current index (%ld) for the number of entries (%ld)", currentIndex, size);
+        return false;
+    }
+
+    if (currentIndex == NoCurrentItemIndex && size) {
+        LOG(SessionState, "WebBackForwardList dictionary representation says there is no current item index, but there is a list of %ld entries - this is bogus", size);
+        return false;
+    }
+    
     BackForwardListItemVector newEntries;
     newEntries.reserveCapacity(size);
     for (CFIndex i = 0; i < size; ++i) {
@@ -142,8 +152,6 @@ bool WebBackForwardList::restoreFromCFDictionaryRepresentation(CFDictionaryRef d
     m_current = currentIndex;
     m_entries = newEntries;
 
-    ASSERT(m_current == NoCurrentItemIndex || m_current < m_entries.size());
-
     return true;
 }