From 3e4a5325aecbbcc578922678b3cada442b49fea1 Mon Sep 17 00:00:00 2001 From: mjs Date: Tue, 7 Dec 2004 00:24:21 +0000 Subject: [PATCH] Reviewed by John. - fixed scripts can cause other frames/windows to execute arbitrary script using javascript: URLs I changed all unprotected places that can navigate a different window or frame from script to check for a javascript: URL, and if found, to check for safety using cross-site-script rules. I considered a few other possible exploits and made no change: - document.location is already protected because the document object itself is protected - frame.src, frame.location, iframe.src and targetted links are all safe because setting the URL of a frame to a javascript: URL executes the script in the context of the parent * khtml/ecma/kjs_window.cpp: (WindowFunc::tryCall): (Location::put): (LocationFunc::tryCall): git-svn-id: https://svn.webkit.org/repository/webkit/trunk@8136 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- WebCore/ChangeLog-2005-08-23 | 24 ++++++++++++++ WebCore/khtml/ecma/kjs_window.cpp | 53 +++++++++++++++++++++---------- 2 files changed, 61 insertions(+), 16 deletions(-) diff --git a/WebCore/ChangeLog-2005-08-23 b/WebCore/ChangeLog-2005-08-23 index 7d038c7827a9..b0ddc595ff22 100644 --- a/WebCore/ChangeLog-2005-08-23 +++ b/WebCore/ChangeLog-2005-08-23 @@ -1,3 +1,27 @@ +2004-12-06 Maciej Stachowiak + + Reviewed by John. + + - fixed scripts can cause other frames/windows to execute arbitrary script using javascript: URLs + + I changed all unprotected places that can navigate a different + window or frame from script to check for a javascript: URL, and if + found, to check for safety using cross-site-script rules. + + I considered a few other possible exploits and made no change: + + - document.location is already protected because the document + object itself is protected + + - frame.src, frame.location, iframe.src and targetted links are + all safe because setting the URL of a frame to a javascript: URL + executes the script in the context of the parent + + * khtml/ecma/kjs_window.cpp: + (WindowFunc::tryCall): + (Location::put): + (LocationFunc::tryCall): + 2004-12-06 Ken Kocienda Reviewed by Maciej diff --git a/WebCore/khtml/ecma/kjs_window.cpp b/WebCore/khtml/ecma/kjs_window.cpp index f1bb0afaf376..0d2808756880 100644 --- a/WebCore/khtml/ecma/kjs_window.cpp +++ b/WebCore/khtml/ecma/kjs_window.cpp @@ -893,7 +893,7 @@ void Window::put(ExecState* exec, const Identifier &propertyName, const Value &v KHTMLPart* p = Window::retrieveActive(exec)->m_part; if (p) { QString dstUrl = p->htmlDocument().completeURL(value.toString(exec).string()).string(); - if (dstUrl.find("javascript:", 0, false) || isSafeScript(exec)) + if (!dstUrl.startsWith("javascript:", false) || isSafeScript(exec)) { bool userGesture = static_cast(exec->dynamicInterpreter())->wasRunByUserGesture(); #if APPLE_CHANGES @@ -1453,8 +1453,12 @@ Value WindowFunc::tryCall(ExecState *exec, Object &thisObj, const List &args) // FIXME: referrer? while ( part->parentPart() ) part = part->parentPart(); - bool userGesture = static_cast(exec->dynamicInterpreter())->wasRunByUserGesture(); - part->scheduleLocationChange(url.url(), false/*don't lock history*/, userGesture); + + const Window* window = Window::retrieveWindow(part); + if (!url.url().startsWith("javascript:", false) || (window && window->isSafeScript(exec))) { + bool userGesture = static_cast(exec->dynamicInterpreter())->wasRunByUserGesture(); + part->scheduleLocationChange(url.url(), false/*don't lock history*/, userGesture); + } return Window::retrieve(part); } if ( uargs.frameName == "_parent" ) @@ -1462,8 +1466,12 @@ Value WindowFunc::tryCall(ExecState *exec, Object &thisObj, const List &args) // FIXME: referrer? if ( part->parentPart() ) part = part->parentPart(); - bool userGesture = static_cast(exec->dynamicInterpreter())->wasRunByUserGesture(); - part->scheduleLocationChange(url.url(), false/*don't lock history*/, userGesture); + + const Window* window = Window::retrieveWindow(part); + if (!url.url().startsWith("javascript:", false) || (window && window->isSafeScript(exec))) { + bool userGesture = static_cast(exec->dynamicInterpreter())->wasRunByUserGesture(); + part->scheduleLocationChange(url.url(), false/*don't lock history*/, userGesture); + } return Window::retrieve(part); } uargs.serviceType = "text/html"; @@ -1495,9 +1503,12 @@ Value WindowFunc::tryCall(ExecState *exec, Object &thisObj, const List &args) } #if APPLE_CHANGES if (!url.isEmpty()) { - bool userGesture = static_cast(exec->dynamicInterpreter())->wasRunByUserGesture(); - // FIXME: Need to pass referrer here. - khtmlpart->scheduleLocationChange(url.url(), false, userGesture); + const Window* window = Window::retrieveWindow(part); + if (!url.url().startsWith("javascript:", false) || (window && window->isSafeScript(exec))) { + bool userGesture = static_cast(exec->dynamicInterpreter())->wasRunByUserGesture(); + // FIXME: Need to pass referrer here. + khtmlpart->scheduleLocationChange(url.url(), false, userGesture); + } } #else uargs.serviceType = QString::null; @@ -2121,13 +2132,17 @@ void Location::put(ExecState *exec, const Identifier &p, const Value &v, int att ObjectImp::put(exec, p, v, attr); return; } - bool userGesture = static_cast(exec->dynamicInterpreter())->wasRunByUserGesture(); + + const Window* window = Window::retrieveWindow(m_part); + if (!url.url().startsWith("javascript:", false) || (window && window->isSafeScript(exec))) { + bool userGesture = static_cast(exec->dynamicInterpreter())->wasRunByUserGesture(); #if APPLE_CHANGES - // We want a new history item if this JS was called via a user gesture - m_part->scheduleLocationChange(url.url(), !userGesture, userGesture); + // We want a new history item if this JS was called via a user gesture + m_part->scheduleLocationChange(url.url(), !userGesture, userGesture); #else - m_part->scheduleLocationChange(url.url(), false /*don't lock history*/, userGesture); + m_part->scheduleLocationChange(url.url(), false /*don't lock history*/, userGesture); #endif + } } Value Location::toPrimitive(ExecState *exec, Type) const @@ -2164,15 +2179,21 @@ Value LocationFunc::tryCall(ExecState *exec, Object &thisObj, const List &args) QString str = args[0].toString(exec).qstring(); KHTMLPart* p = Window::retrieveActive(exec)->part(); if ( p ) { - bool userGesture = static_cast(exec->dynamicInterpreter())->wasRunByUserGesture(); - part->scheduleLocationChange(p->htmlDocument().completeURL(str).string(), true /*lock history*/, userGesture); + const Window* window = Window::retrieveWindow(part); + if (!str.startsWith("javascript:", false) || (window && window->isSafeScript(exec))) { + bool userGesture = static_cast(exec->dynamicInterpreter())->wasRunByUserGesture(); + part->scheduleLocationChange(p->htmlDocument().completeURL(str).string(), true /*lock history*/, userGesture); + } } break; } case Location::Reload: { - bool userGesture = static_cast(exec->dynamicInterpreter())->wasRunByUserGesture(); - part->scheduleLocationChange(part->url().url(), true/*lock history*/, userGesture); + const Window* window = Window::retrieveWindow(part); + if (!part->url().url().startsWith("javascript:", false) || (window && window->isSafeScript(exec))) { + bool userGesture = static_cast(exec->dynamicInterpreter())->wasRunByUserGesture(); + part->scheduleLocationChange(part->url().url(), true/*lock history*/, userGesture); + } break; } case Location::ToString: -- 2.36.0