Commit 67ea898e authored by ap@webkit.org's avatar ap@webkit.org

Reviewed by Sam Weinig.

        https://bugs.webkit.org/show_bug.cgi?id=25420
        <rdar://problem/6829570> REGRESSION: XMLHttpRequest allows loading from another origin

        Test: http/tests/xmlhttprequest/detaching-frame-2.html

        This was caused by faulty DOMWindow::document(), which could return a new document from
        the window's frame after navigation.

        * bindings/js/JSDOMWindowCustom.h: (WebCore::JSDOMWindowBase::allowsAccessFromPrivate):
        Removed an obsolete check that allowed access when document was null. Contrary to what a
        comment said, that can happen for a window that is no longer in frame, not to one whose
        document is not constructed yet.

        * bindings/js/JSXMLHttpRequestConstructor.cpp: (WebCore::constructXMLHttpRequest): Bail
        out if context was not found. This currently happens due to a shortcoming in
        DOMWindow::document() - when it is fixed, the XMLHttpRequest object in included regression
        test will be constructed successfully, but won't be sent, because its context will be
        frameless.

        * page/DOMWindow.cpp: (WebCore::DOMWindow::document): Check that the window in frame hasn't
        been replaced yet. Added FIXME comments about how this may be better fixed in the future.

        * bindings/js/JSAudioConstructor.cpp:
        (WebCore::JSAudioConstructor::document):
        (WebCore::constructAudio):
        * bindings/js/JSImageConstructor.cpp:
        (WebCore::JSImageConstructor::document):
        (WebCore::constructImage):
        * bindings/js/JSMessageChannelConstructor.cpp:
        (WebCore::JSMessageChannelConstructor::construct):
        * bindings/js/JSOptionConstructor.cpp:
        (WebCore::JSOptionConstructor::document):
        (WebCore::constructHTMLOptionElement):
        Make matching changes to other constructors that hold a reference to global object.



git-svn-id: svn://svn.chromium.org/blink/trunk@42983 bbb929c8-8fbe-4397-9dbb-9b2b20218538
parent e8420b29
2009-04-28 Alexey Proskuryakov <ap@webkit.org>
Reviewed by Sam Weinig.
https://bugs.webkit.org/show_bug.cgi?id=25420
<rdar://problem/6829570> REGRESSION: XMLHttpRequest allows loading from another origin
The test in intentionally vague, as the behavior is not fully specified yet, and any way to
prevent cross-origin load is fine for now.
* http/tests/xmlhttprequest/detaching-frame-2-expected.txt: Added.
* http/tests/xmlhttprequest/detaching-frame-2.html: Added.
* http/tests/xmlhttprequest/resources/echo-host.php: Added.
2009-04-28 Eric Seidel <eric@webkit.org> 2009-04-28 Eric Seidel <eric@webkit.org>
Reviewed by Simon Fraser. Reviewed by Simon Fraser.
<!doctype html>
<body>
<p><a href="https://bugs.webkit.org/show_bug.cgi?id=25420">bug 25240</a></p>
<script>
if (window.layoutTestController) {
layoutTestController.dumpAsText()
layoutTestController.waitUntilDone()
}
function test() {
try {
window.x = frames[0].XMLHttpRequest
} catch(e) { alert(e) }
frames[0].frameElement.onload = function() {
var client
try {
client = new XMLHttpRequest()
client.open("GET", "/xmlhttprequest/resources/meaningless-frame.html", false)
client.send("")
} catch(e)
{
}
// FAIL if loaded data from another domain.
document.getElementById("result").innerHTML = (client && client.responseText.match(/localhost/)) ? "FAIL" : "PASS"
if (window.layoutTestController)
layoutTestController.notifyDone()
}
frames[0].location = frames[0].location.toString().replace("127.0.0.1", "localhost")
}
</script>
<iframe src="resources/echo-host.php" onload="test()"></iframe>
<div id=result>FAIL: Script did not run.</div>
</body>
2009-04-28 Alexey Proskuryakov <ap@webkit.org>
Reviewed by Sam Weinig.
https://bugs.webkit.org/show_bug.cgi?id=25420
<rdar://problem/6829570> REGRESSION: XMLHttpRequest allows loading from another origin
Test: http/tests/xmlhttprequest/detaching-frame-2.html
This was caused by faulty DOMWindow::document(), which could return a new document from
the window's frame after navigation.
* bindings/js/JSDOMWindowCustom.h: (WebCore::JSDOMWindowBase::allowsAccessFromPrivate):
Removed an obsolete check that allowed access when document was null. Contrary to what a
comment said, that can happen for a window that is no longer in frame, not to one whose
document is not constructed yet.
* bindings/js/JSXMLHttpRequestConstructor.cpp: (WebCore::constructXMLHttpRequest): Bail
out if context was not found. This currently happens due to a shortcoming in
DOMWindow::document() - when it is fixed, the XMLHttpRequest object in included regression
test will be constructed successfully, but won't be sent, because its context will be
frameless.
* page/DOMWindow.cpp: (WebCore::DOMWindow::document): Check that the window in frame hasn't
been replaced yet. Added FIXME comments about how this may be better fixed in the future.
* bindings/js/JSAudioConstructor.cpp:
(WebCore::JSAudioConstructor::document):
(WebCore::constructAudio):
* bindings/js/JSImageConstructor.cpp:
(WebCore::JSImageConstructor::document):
(WebCore::constructImage):
* bindings/js/JSMessageChannelConstructor.cpp:
(WebCore::JSMessageChannelConstructor::construct):
* bindings/js/JSOptionConstructor.cpp:
(WebCore::JSOptionConstructor::document):
(WebCore::constructHTMLOptionElement):
Make matching changes to other constructors that hold a reference to global object.
2009-04-28 Kevin Ollivier <kevino@theolliviers.com> 2009-04-28 Kevin Ollivier <kevino@theolliviers.com>
wxMSW build fix. Switch JSCore build back to static. wxMSW build fix. Switch JSCore build back to static.
...@@ -61,7 +61,11 @@ static JSObject* constructAudio(ExecState* exec, JSObject* constructor, const Ar ...@@ -61,7 +61,11 @@ static JSObject* constructAudio(ExecState* exec, JSObject* constructor, const Ar
{ {
// FIXME: Why doesn't this need the call toJS on the document like JSImageConstructor? // FIXME: Why doesn't this need the call toJS on the document like JSImageConstructor?
RefPtr<HTMLAudioElement> audio = new HTMLAudioElement(HTMLNames::audioTag, static_cast<JSAudioConstructor*>(constructor)->document()); Document* document = static_cast<JSAudioConstructor*>(constructor)->document();
if (!document)
return throwError(exec, ReferenceError, "Audio constructor associated document is unavailable");
RefPtr<HTMLAudioElement> audio = new HTMLAudioElement(HTMLNames::audioTag, document);
if (args.size() > 0) { if (args.size() > 0) {
audio->setSrc(args.at(exec, 0).toString(exec)); audio->setSrc(args.at(exec, 0).toString(exec));
audio->scheduleLoad(); audio->scheduleLoad();
......
...@@ -188,12 +188,6 @@ ALWAYS_INLINE bool JSDOMWindowBase::allowsAccessFromPrivate(const JSGlobalObject ...@@ -188,12 +188,6 @@ ALWAYS_INLINE bool JSDOMWindowBase::allowsAccessFromPrivate(const JSGlobalObject
if (originWindow == targetWindow) if (originWindow == targetWindow)
return true; return true;
// JS may be attempting to access the "window" object, which should be valid,
// even if the document hasn't been constructed yet. If the document doesn't
// exist yet allow JS to access the window object.
if (!originWindow->impl()->document())
return true;
const SecurityOrigin* originSecurityOrigin = originWindow->impl()->securityOrigin(); const SecurityOrigin* originSecurityOrigin = originWindow->impl()->securityOrigin();
const SecurityOrigin* targetSecurityOrigin = targetWindow->impl()->securityOrigin(); const SecurityOrigin* targetSecurityOrigin = targetWindow->impl()->securityOrigin();
......
...@@ -65,6 +65,8 @@ static JSObject* constructImage(ExecState* exec, JSObject* constructor, const Ar ...@@ -65,6 +65,8 @@ static JSObject* constructImage(ExecState* exec, JSObject* constructor, const Ar
} }
Document* document = static_cast<JSImageConstructor*>(constructor)->document(); Document* document = static_cast<JSImageConstructor*>(constructor)->document();
if (!document)
return throwError(exec, ReferenceError, "Image constructor associated document is unavailable");
// Calling toJS on the document causes the JS document wrapper to be // Calling toJS on the document causes the JS document wrapper to be
// added to the window object. This is done to ensure that JSDocument::mark // added to the window object. This is done to ensure that JSDocument::mark
......
...@@ -61,7 +61,11 @@ ConstructType JSMessageChannelConstructor::getConstructData(ConstructData& const ...@@ -61,7 +61,11 @@ ConstructType JSMessageChannelConstructor::getConstructData(ConstructData& const
JSObject* JSMessageChannelConstructor::construct(ExecState* exec, JSObject* constructor, const ArgList&) JSObject* JSMessageChannelConstructor::construct(ExecState* exec, JSObject* constructor, const ArgList&)
{ {
return asObject(toJS(exec, MessageChannel::create(static_cast<JSMessageChannelConstructor*>(constructor)->scriptExecutionContext()))); ScriptExecutionContext* context = static_cast<JSMessageChannelConstructor*>(constructor)->scriptExecutionContext();
if (!context)
return throwError(exec, ReferenceError, "MessageChannel constructor associated document is unavailable");
return asObject(toJS(exec, MessageChannel::create(context)));
} }
void JSMessageChannelConstructor::mark() void JSMessageChannelConstructor::mark()
......
...@@ -53,6 +53,8 @@ Document* JSOptionConstructor::document() const ...@@ -53,6 +53,8 @@ Document* JSOptionConstructor::document() const
static JSObject* constructHTMLOptionElement(ExecState* exec, JSObject* constructor, const ArgList& args) static JSObject* constructHTMLOptionElement(ExecState* exec, JSObject* constructor, const ArgList& args)
{ {
Document* document = static_cast<JSOptionConstructor*>(constructor)->document(); Document* document = static_cast<JSOptionConstructor*>(constructor)->document();
if (!document)
return throwError(exec, ReferenceError, "Option constructor associated document is unavailable");
RefPtr<HTMLOptionElement> element = static_pointer_cast<HTMLOptionElement>(document->createElement(HTMLNames::optionTag, false)); RefPtr<HTMLOptionElement> element = static_pointer_cast<HTMLOptionElement>(document->createElement(HTMLNames::optionTag, false));
......
...@@ -46,7 +46,11 @@ ScriptExecutionContext* JSXMLHttpRequestConstructor::scriptExecutionContext() co ...@@ -46,7 +46,11 @@ ScriptExecutionContext* JSXMLHttpRequestConstructor::scriptExecutionContext() co
static JSObject* constructXMLHttpRequest(ExecState* exec, JSObject* constructor, const ArgList&) static JSObject* constructXMLHttpRequest(ExecState* exec, JSObject* constructor, const ArgList&)
{ {
RefPtr<XMLHttpRequest> xmlHttpRequest = XMLHttpRequest::create(static_cast<JSXMLHttpRequestConstructor*>(constructor)->scriptExecutionContext()); ScriptExecutionContext* context = static_cast<JSXMLHttpRequestConstructor*>(constructor)->scriptExecutionContext();
if (!context)
return throwError(exec, ReferenceError, "XMLHttpRequest constructor associated document is unavailable");
RefPtr<XMLHttpRequest> xmlHttpRequest = XMLHttpRequest::create(context);
return CREATE_DOM_OBJECT_WRAPPER(exec, XMLHttpRequest, xmlHttpRequest.get()); return CREATE_DOM_OBJECT_WRAPPER(exec, XMLHttpRequest, xmlHttpRequest.get());
} }
......
...@@ -977,9 +977,15 @@ DOMWindow* DOMWindow::top() const ...@@ -977,9 +977,15 @@ DOMWindow* DOMWindow::top() const
Document* DOMWindow::document() const Document* DOMWindow::document() const
{ {
// FIXME: This function shouldn't need a frame to work.
if (!m_frame) if (!m_frame)
return 0; return 0;
// The m_frame pointer is not zeroed out when the window is put into b/f cache, so it can hold an unrelated document/window pair.
// FIXME: We should always zero out the frame pointer on navigation to avoid accidentally accessing the new frame content.
if (m_frame->domWindow() != this)
return 0;
ASSERT(m_frame->document()); ASSERT(m_frame->document());
return m_frame->document(); return m_frame->document();
} }
......
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