Diff
Modified: branches/safari-536.30-branch/LayoutTests/ChangeLog (148339 => 148340)
--- branches/safari-536.30-branch/LayoutTests/ChangeLog 2013-04-13 02:38:50 UTC (rev 148339)
+++ branches/safari-536.30-branch/LayoutTests/ChangeLog 2013-04-13 03:09:57 UTC (rev 148340)
@@ -1,3 +1,22 @@
+2013-04-12 Ryosuke Niwa <[email protected]>
+
+ Merge r140748.
+
+ 2013-01-24 Kentaro Hara <[email protected]>
+
+ Regression(r107058): Use-after-free in SerializedScriptValue::deserialize
+ https://bugs.webkit.org/show_bug.cgi?id=107792
+
+ Reviewed by Abhishek Arya.
+
+ Added a test that demonstrated a crash due to use-after-free
+ of SerializedScriptValue.
+
+ Test: fast/history/replacestate-nocrash.html
+
+ * fast/history/replacestate-nocrash-expected.txt: Added.
+ * fast/history/replacestate-nocrash.html: Added.
+
2013-04-12 Lucas Forschler <[email protected]>
Merge r136596
Copied: branches/safari-536.30-branch/LayoutTests/fast/history/replacestate-nocrash-expected.txt (from rev 140748, trunk/LayoutTests/fast/history/replacestate-nocrash-expected.txt) (0 => 148340)
--- branches/safari-536.30-branch/LayoutTests/fast/history/replacestate-nocrash-expected.txt (rev 0)
+++ branches/safari-536.30-branch/LayoutTests/fast/history/replacestate-nocrash-expected.txt 2013-04-13 03:09:57 UTC (rev 148340)
@@ -0,0 +1 @@
+Test passes if it does not crash.
Copied: branches/safari-536.30-branch/LayoutTests/fast/history/replacestate-nocrash.html (from rev 140748, trunk/LayoutTests/fast/history/replacestate-nocrash.html) (0 => 148340)
--- branches/safari-536.30-branch/LayoutTests/fast/history/replacestate-nocrash.html (rev 0)
+++ branches/safari-536.30-branch/LayoutTests/fast/history/replacestate-nocrash.html 2013-04-13 03:09:57 UTC (rev 148340)
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<html>
+Test passes if it does not crash.
+<script>
+if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+Object.prototype.__defineSetter__("foo",function(){history.replaceState("")});
+history.replaceState({foo:1,zzz:Array(1<<22).join("a")});
+history.state.length;
+</script>
+</html>
Modified: branches/safari-536.30-branch/Source/WebCore/ChangeLog (148339 => 148340)
--- branches/safari-536.30-branch/Source/WebCore/ChangeLog 2013-04-13 02:38:50 UTC (rev 148339)
+++ branches/safari-536.30-branch/Source/WebCore/ChangeLog 2013-04-13 03:09:57 UTC (rev 148340)
@@ -1,3 +1,48 @@
+2013-04-12 Ryosuke Niwa <[email protected]>
+
+ Merge r140748.
+
+ 2013-01-24 Kentaro Hara <[email protected]>
+
+ Regression(r107058): Use-after-free in SerializedScriptValue::deserialize
+ https://bugs.webkit.org/show_bug.cgi?id=107792
+
+ Reviewed by Abhishek Arya.
+
+ Imagine the following call path:
+
+ (1) history.state is accessed.
+ (2) V8History::stateAccessorGetter() calls History::state(), which calls
+ HistoryItem::stateObject().
+ (3) HistoryItem holds m_stateObject as RefPtr<SerializedScriptValue>,
+ but HistoryItem::stateObject() returns SerializedScriptValue*.
+ (4) V8History::stateAccessorGetter calls SerializedScriptValue::deserialize()
+ for the SerializedScriptValue* obtained in (3).
+ (5) SerializedScriptValue::deserialize() can call history.replaceState()
+ in its deserialization process (See the test case in the Chromium bug).
+ (6) history.replaceState() replaces HistoryItem::m_stateObject.
+ This replacement destructs the original HistoryItem::m_stateObject.
+ (7) The current deserialization process can crash due to the premature destruction.
+
+ To avoid the problem, we have to pass PassRefPtr<SerializedScriptValue> around
+ instead of SerializedScriptValue*.
+
+ Test: fast/history/replacestate-nocrash.html
+
+ * bindings/v8/custom/V8HistoryCustom.cpp:
+ (WebCore::V8History::stateAccessorGetter):
+ * history/HistoryItem.h:
+ (WebCore):
+ (WebCore::HistoryItem::stateObject):
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::loadInSameDocument):
+ * loader/FrameLoader.h:
+ * page/History.cpp:
+ (WebCore::History::state):
+ (WebCore::History::stateInternal):
+ * page/History.h:
+ (History):
+
2013-04-12 Lucas Forschler <[email protected]>
Merge r129814
Modified: branches/safari-536.30-branch/Source/WebCore/bindings/js/JSHistoryCustom.cpp (148339 => 148340)
--- branches/safari-536.30-branch/Source/WebCore/bindings/js/JSHistoryCustom.cpp 2013-04-13 02:38:50 UTC (rev 148339)
+++ branches/safari-536.30-branch/Source/WebCore/bindings/js/JSHistoryCustom.cpp 2013-04-13 03:09:57 UTC (rev 148340)
@@ -172,7 +172,7 @@
if (!cachedValue.isEmpty() && !history->stateChanged())
return cachedValue;
- SerializedScriptValue* serialized = history->state();
+ RefPtr<SerializedScriptValue> serialized = history->state();
JSValue result = serialized ? serialized->deserialize(exec, globalObject(), 0) : jsNull();
const_cast<JSHistory*>(this)->m_state.set(exec->globalData(), this, result);
return result;
Modified: branches/safari-536.30-branch/Source/WebCore/bindings/v8/custom/V8HistoryCustom.cpp (148339 => 148340)
--- branches/safari-536.30-branch/Source/WebCore/bindings/v8/custom/V8HistoryCustom.cpp 2013-04-13 02:38:50 UTC (rev 148339)
+++ branches/safari-536.30-branch/Source/WebCore/bindings/v8/custom/V8HistoryCustom.cpp 2013-04-13 03:09:57 UTC (rev 148340)
@@ -52,7 +52,7 @@
if (!value.IsEmpty() && !history->stateChanged())
return value;
- SerializedScriptValue* serialized = history->state();
+ RefPtr<SerializedScriptValue> serialized = history->state();
value = serialized ? serialized->deserialize(0, info.GetIsolate()) : v8::Handle<v8::Value>(v8::Null());
info.Holder()->SetHiddenValue(V8HiddenPropertyName::state(), value);
Modified: branches/safari-536.30-branch/Source/WebCore/history/HistoryItem.h (148339 => 148340)
--- branches/safari-536.30-branch/Source/WebCore/history/HistoryItem.h 2013-04-13 02:38:50 UTC (rev 148339)
+++ branches/safari-536.30-branch/Source/WebCore/history/HistoryItem.h 2013-04-13 03:09:57 UTC (rev 148340)
@@ -29,6 +29,7 @@
#include "IntPoint.h"
#include "PlatformString.h"
+#include "SerializedScriptValue.h"
#include <wtf/HashMap.h>
#include <wtf/OwnPtr.h>
#include <wtf/PassOwnPtr.h>
@@ -58,7 +59,6 @@
class Image;
class KURL;
class ResourceRequest;
-class SerializedScriptValue;
typedef Vector<RefPtr<HistoryItem> > HistoryItemVector;
@@ -145,7 +145,7 @@
void setIsTargetItem(bool);
void setStateObject(PassRefPtr<SerializedScriptValue> object);
- SerializedScriptValue* stateObject() const { return m_stateObject.get(); }
+ PassRefPtr<SerializedScriptValue> stateObject() const { return m_stateObject; }
void setItemSequenceNumber(long long number) { m_itemSequenceNumber = number; }
long long itemSequenceNumber() const { return m_itemSequenceNumber; }
Modified: branches/safari-536.30-branch/Source/WebCore/loader/FrameLoader.cpp (148339 => 148340)
--- branches/safari-536.30-branch/Source/WebCore/loader/FrameLoader.cpp 2013-04-13 02:38:50 UTC (rev 148339)
+++ branches/safari-536.30-branch/Source/WebCore/loader/FrameLoader.cpp 2013-04-13 03:09:57 UTC (rev 148340)
@@ -968,7 +968,7 @@
// This does the same kind of work that didOpenURL does, except it relies on the fact
// that a higher level already checked that the URLs match and the scrolling is the right thing to do.
-void FrameLoader::loadInSameDocument(const KURL& url, SerializedScriptValue* stateObject, bool isNewNavigation)
+void FrameLoader::loadInSameDocument(const KURL& url, PassRefPtr<SerializedScriptValue> stateObject, bool isNewNavigation)
{
// If we have a state object, we cannot also be a new navigation.
ASSERT(!stateObject || (stateObject && !isNewNavigation));
@@ -1021,7 +1021,7 @@
m_client->dispatchDidNavigateWithinPage();
- m_frame->document()->statePopped(stateObject ? stateObject : SerializedScriptValue::nullValue());
+ m_frame->document()->statePopped(stateObject ? stateObject.get() : SerializedScriptValue::nullValue());
m_client->dispatchDidPopStateWithinPage();
if (hashChange) {
Modified: branches/safari-536.30-branch/Source/WebCore/loader/FrameLoader.h (148339 => 148340)
--- branches/safari-536.30-branch/Source/WebCore/loader/FrameLoader.h 2013-04-13 02:38:50 UTC (rev 148339)
+++ branches/safari-536.30-branch/Source/WebCore/loader/FrameLoader.h 2013-04-13 03:09:57 UTC (rev 148340)
@@ -351,7 +351,7 @@
void detachChildren();
void closeAndRemoveChild(Frame*);
- void loadInSameDocument(const KURL&, SerializedScriptValue* stateObject, bool isNewNavigation);
+ void loadInSameDocument(const KURL&, PassRefPtr<SerializedScriptValue> stateObject, bool isNewNavigation);
void prepareForLoadStart();
void provisionalLoadStarted();
Modified: branches/safari-536.30-branch/Source/WebCore/page/History.cpp (148339 => 148340)
--- branches/safari-536.30-branch/Source/WebCore/page/History.cpp 2013-04-13 02:38:50 UTC (rev 148339)
+++ branches/safari-536.30-branch/Source/WebCore/page/History.cpp 2013-04-13 03:09:57 UTC (rev 148340)
@@ -55,13 +55,13 @@
return m_frame->page()->backForward()->count();
}
-SerializedScriptValue* History::state()
+PassRefPtr<SerializedScriptValue> History::state()
{
m_lastStateObjectRequested = stateInternal();
return m_lastStateObjectRequested;
}
-SerializedScriptValue* History::stateInternal() const
+PassRefPtr<SerializedScriptValue> History::stateInternal() const
{
if (!m_frame)
return 0;
Modified: branches/safari-536.30-branch/Source/WebCore/page/History.h (148339 => 148340)
--- branches/safari-536.30-branch/Source/WebCore/page/History.h 2013-04-13 02:38:50 UTC (rev 148339)
+++ branches/safari-536.30-branch/Source/WebCore/page/History.h 2013-04-13 03:09:57 UTC (rev 148340)
@@ -28,6 +28,7 @@
#include "DOMWindowProperty.h"
#include "KURL.h"
+#include "SerializedScriptValue.h"
#include <wtf/Forward.h>
#include <wtf/PassRefPtr.h>
#include <wtf/RefCounted.h>
@@ -44,7 +45,7 @@
static PassRefPtr<History> create(Frame* frame) { return adoptRef(new History(frame)); }
unsigned length() const;
- SerializedScriptValue* state();
+ PassRefPtr<SerializedScriptValue> state();
void back();
void forward();
void go(int distance);
@@ -67,9 +68,9 @@
KURL urlForState(const String& url);
- SerializedScriptValue* stateInternal() const;
+ PassRefPtr<SerializedScriptValue> stateInternal() const;
- SerializedScriptValue* m_lastStateObjectRequested;
+ RefPtr<SerializedScriptValue> m_lastStateObjectRequested;
};
} // namespace WebCore
