Commit 162c7392 authored by haraken@chromium.org's avatar haraken@chromium.org

Regression(r107058): Use-after-free in SerializedScriptValue::deserialize

https://bugs.webkit.org/show_bug.cgi?id=107792

Reviewed by Abhishek Arya.

Source/WebCore:

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):

LayoutTests:

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.


git-svn-id: http://svn.webkit.org/repository/webkit/trunk@140748 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 46045b5d
2013-01-24 Kentaro Hara <haraken@chromium.org>
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-01-24 Arko Saha <arko@motorola.com>
Microdata: itemtype attribute must update correctly on adding or removing tokens
<!DOCTYPE html>
<html>
Test passes if it does not crash.
<script>
if (window.testRunner)
testRunner.dumpAsText();
Object.prototype.__defineSetter__("foo",function(){history.replaceState("")});
history.replaceState({foo:1,zzz:Array(1<<22).join("a")});
history.state.length;
</script>
</html>
2013-01-24 Kentaro Hara <haraken@chromium.org>
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-01-24 Beth Dakin <bdakin@apple.com>
Some formerly-fixed objects scroll as if they are still fixed
......@@ -50,7 +50,7 @@ v8::Handle<v8::Value> V8History::stateAccessorGetter(v8::Local<v8::String> name,
if (!value.IsEmpty() && !history->stateChanged())
return value;
SerializedScriptValue* serialized = history->state();
RefPtr<SerializedScriptValue> serialized = history->state();
value = serialized ? serialized->deserialize(info.GetIsolate()) : v8::Handle<v8::Value>(v8Null(info.GetIsolate()));
info.Holder()->SetHiddenValue(V8HiddenPropertyName::state(), value);
......
......@@ -28,6 +28,7 @@
#define HistoryItem_h
#include "IntPoint.h"
#include "SerializedScriptValue.h"
#include <wtf/HashMap.h>
#include <wtf/OwnPtr.h>
#include <wtf/PassOwnPtr.h>
......@@ -58,7 +59,6 @@ class HistoryItem;
class Image;
class KURL;
class ResourceRequest;
class SerializedScriptValue;
typedef Vector<RefPtr<HistoryItem> > HistoryItemVector;
......@@ -145,7 +145,7 @@ public:
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; }
......
......@@ -1003,7 +1003,7 @@ void FrameLoader::setFirstPartyForCookies(const KURL& url)
// 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));
......
......@@ -359,7 +359,7 @@ private:
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();
......
......@@ -55,13 +55,13 @@ unsigned History::length() const
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;
......
......@@ -45,7 +45,7 @@ public:
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);
......@@ -68,9 +68,9 @@ private:
KURL urlForState(const String& url);
SerializedScriptValue* stateInternal() const;
PassRefPtr<SerializedScriptValue> stateInternal() const;
SerializedScriptValue* m_lastStateObjectRequested;
RefPtr<SerializedScriptValue> m_lastStateObjectRequested;
};
} // namespace WebCore
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment