[SCM] WebKit Debian packaging branch, webkit-1.1, updated. upstream/1.1.15.1-1414-gc69ee75
abarth at webkit.org
abarth at webkit.org
Thu Oct 29 20:31:43 UTC 2009
The following commit has been merged in the webkit-1.1 branch:
commit bd7401884a18899c6d59c58f40a9129c8213aa8b
Author: abarth at webkit.org <abarth at webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Tue Sep 22 05:08:37 2009 +0000
2009-09-21 Adam Barth <abarth at webkit.org>
Reviewed by Sam Weinig.
Don't re-enter JavaScript after performing access checks
https://bugs.webkit.org/show_bug.cgi?id=29531
Moved the access check slightly later in this functions to avoid
re-entering the JavaScript interpreter (typically via toString)
after performing the access check.
I can't really think of a meaningful test for this change. It's more
security hygiene.
* bindings/js/JSDOMWindowCustom.cpp:
(WebCore::JSDOMWindow::setLocation):
(WebCore::JSDOMWindow::open):
(WebCore::JSDOMWindow::showModalDialog):
* bindings/js/JSLocationCustom.cpp:
(WebCore::JSLocation::setHref):
(WebCore::JSLocation::replace):
(WebCore::JSLocation::assign):
* bindings/v8/custom/V8DOMWindowCustom.cpp:
(WebCore::V8Custom::WindowSetTimeoutImpl):
(WebCore::if):
(CALLBACK_FUNC_DECL):
(V8Custom::WindowSetLocation):
(V8Custom::ClearTimeoutImpl):
* bindings/v8/custom/V8LocationCustom.cpp:
(WebCore::ACCESSOR_SETTER):
(WebCore::CALLBACK_FUNC_DECL):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@48619 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 457fb4a..5c4b58f 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,35 @@
+2009-09-21 Adam Barth <abarth at webkit.org>
+
+ Reviewed by Sam Weinig.
+
+ Don't re-enter JavaScript after performing access checks
+ https://bugs.webkit.org/show_bug.cgi?id=29531
+
+ Moved the access check slightly later in this functions to avoid
+ re-entering the JavaScript interpreter (typically via toString)
+ after performing the access check.
+
+ I can't really think of a meaningful test for this change. It's more
+ security hygiene.
+
+ * bindings/js/JSDOMWindowCustom.cpp:
+ (WebCore::JSDOMWindow::setLocation):
+ (WebCore::JSDOMWindow::open):
+ (WebCore::JSDOMWindow::showModalDialog):
+ * bindings/js/JSLocationCustom.cpp:
+ (WebCore::JSLocation::setHref):
+ (WebCore::JSLocation::replace):
+ (WebCore::JSLocation::assign):
+ * bindings/v8/custom/V8DOMWindowCustom.cpp:
+ (WebCore::V8Custom::WindowSetTimeoutImpl):
+ (WebCore::if):
+ (CALLBACK_FUNC_DECL):
+ (V8Custom::WindowSetLocation):
+ (V8Custom::ClearTimeoutImpl):
+ * bindings/v8/custom/V8LocationCustom.cpp:
+ (WebCore::ACCESSOR_SETTER):
+ (WebCore::CALLBACK_FUNC_DECL):
+
2009-09-21 Dumitru Daniliuc <dumi at chromium.org>
Reviewed by Eric Seidel.
diff --git a/WebCore/bindings/js/JSDOMWindowCustom.cpp b/WebCore/bindings/js/JSDOMWindowCustom.cpp
index 7fe1dc5..7217fc8 100644
--- a/WebCore/bindings/js/JSDOMWindowCustom.cpp
+++ b/WebCore/bindings/js/JSDOMWindowCustom.cpp
@@ -580,13 +580,13 @@ void JSDOMWindow::setLocation(ExecState* exec, JSValue value)
Frame* frame = impl()->frame();
ASSERT(frame);
- if (!shouldAllowNavigation(exec, frame))
- return;
-
KURL url = completeURL(exec, value.toString(exec));
if (url.isNull())
return;
+ if (!shouldAllowNavigation(exec, frame))
+ return;
+
if (!protocolIsJavaScript(url) || allowsAccessFrom(exec)) {
// We want a new history item if this JS was called via a user gesture
frame->loader()->scheduleLocationChange(url, lexicalFrame->loader()->outgoingReferrer(), !lexicalFrame->script()->anyPageIsProcessingUserGesture(), false, processingUserGesture(exec));
@@ -781,6 +781,10 @@ static Frame* createWindow(ExecState* exec, Frame* lexicalFrame, Frame* dynamicF
JSValue JSDOMWindow::open(ExecState* exec, const ArgList& args)
{
+ String urlString = valueToStringWithUndefinedOrNullCheck(exec, args.at(0));
+ AtomicString frameName = args.at(1).isUndefinedOrNull() ? "_blank" : AtomicString(args.at(1).toString(exec));
+ WindowFeatures windowFeatures(valueToStringWithUndefinedOrNullCheck(exec, args.at(2)));
+
Frame* frame = impl()->frame();
if (!frame)
return jsUndefined();
@@ -793,9 +797,6 @@ JSValue JSDOMWindow::open(ExecState* exec, const ArgList& args)
Page* page = frame->page();
- String urlString = valueToStringWithUndefinedOrNullCheck(exec, args.at(0));
- AtomicString frameName = args.at(1).isUndefinedOrNull() ? "_blank" : AtomicString(args.at(1).toString(exec));
-
// Because FrameTree::find() returns true for empty strings, we must check for empty framenames.
// Otherwise, illegitimate window.open() calls with no name will pass right through the popup blocker.
if (!DOMWindow::allowPopUp(dynamicFrame) && (frameName.isEmpty() || !frame->tree()->find(frameName)))
@@ -813,13 +814,13 @@ JSValue JSDOMWindow::open(ExecState* exec, const ArgList& args)
topOrParent = true;
}
if (topOrParent) {
- if (!shouldAllowNavigation(exec, frame))
- return jsUndefined();
-
String completedURL;
if (!urlString.isEmpty())
completedURL = completeURL(exec, urlString).string();
+ if (!shouldAllowNavigation(exec, frame))
+ return jsUndefined();
+
const JSDOMWindow* targetedWindow = toJSDOMWindow(frame);
if (!completedURL.isEmpty() && (!protocolIsJavaScript(completedURL) || (targetedWindow && targetedWindow->allowsAccessFrom(exec)))) {
bool userGesture = processingUserGesture(exec);
@@ -835,7 +836,6 @@ JSValue JSDOMWindow::open(ExecState* exec, const ArgList& args)
}
// In the case of a named frame or a new window, we'll use the createWindow() helper
- WindowFeatures windowFeatures(valueToStringWithUndefinedOrNullCheck(exec, args.at(2)));
FloatRect windowRect(windowFeatures.xSet ? windowFeatures.x : 0, windowFeatures.ySet ? windowFeatures.y : 0,
windowFeatures.widthSet ? windowFeatures.width : 0, windowFeatures.heightSet ? windowFeatures.height : 0);
DOMWindow::adjustWindowRect(screenAvailableRect(page ? page->mainFrame()->view() : 0), windowRect, windowRect);
@@ -855,6 +855,10 @@ JSValue JSDOMWindow::open(ExecState* exec, const ArgList& args)
JSValue JSDOMWindow::showModalDialog(ExecState* exec, const ArgList& args)
{
+ String url = valueToStringWithUndefinedOrNullCheck(exec, args.at(0));
+ JSValue dialogArgs = args.at(1);
+ String featureArgs = valueToStringWithUndefinedOrNullCheck(exec, args.at(2));
+
Frame* frame = impl()->frame();
if (!frame)
return jsUndefined();
@@ -868,10 +872,6 @@ JSValue JSDOMWindow::showModalDialog(ExecState* exec, const ArgList& args)
if (!DOMWindow::canShowModalDialogNow(frame) || !DOMWindow::allowPopUp(dynamicFrame))
return jsUndefined();
- String url = valueToStringWithUndefinedOrNullCheck(exec, args.at(0));
- JSValue dialogArgs = args.at(1);
- String featureArgs = valueToStringWithUndefinedOrNullCheck(exec, args.at(2));
-
HashMap<String, String> features;
DOMWindow::parseModalDialogFeatures(featureArgs, features);
diff --git a/WebCore/bindings/js/JSLocationCustom.cpp b/WebCore/bindings/js/JSLocationCustom.cpp
index 9cd942c..aecec5e 100644
--- a/WebCore/bindings/js/JSLocationCustom.cpp
+++ b/WebCore/bindings/js/JSLocationCustom.cpp
@@ -204,13 +204,13 @@ void JSLocation::setHref(ExecState* exec, JSValue value)
Frame* frame = impl()->frame();
ASSERT(frame);
- if (!shouldAllowNavigation(exec, frame))
- return;
-
KURL url = completeURL(exec, value.toString(exec));
if (url.isNull())
return;
+ if (!shouldAllowNavigation(exec, frame))
+ return;
+
navigateIfAllowed(exec, frame, url, !frame->script()->anyPageIsProcessingUserGesture(), false);
}
@@ -308,13 +308,13 @@ JSValue JSLocation::replace(ExecState* exec, const ArgList& args)
if (!frame)
return jsUndefined();
- if (!shouldAllowNavigation(exec, frame))
- return jsUndefined();
-
KURL url = completeURL(exec, args.at(0).toString(exec));
if (url.isNull())
return jsUndefined();
+ if (!shouldAllowNavigation(exec, frame))
+ return jsUndefined();
+
navigateIfAllowed(exec, frame, url, true, true);
return jsUndefined();
}
@@ -336,13 +336,13 @@ JSValue JSLocation::assign(ExecState* exec, const ArgList& args)
if (!frame)
return jsUndefined();
- if (!shouldAllowNavigation(exec, frame))
- return jsUndefined();
-
KURL url = completeURL(exec, args.at(0).toString(exec));
if (url.isNull())
return jsUndefined();
+ if (!shouldAllowNavigation(exec, frame))
+ return jsUndefined();
+
// We want a new history item if this JS was called via a user gesture
navigateIfAllowed(exec, frame, url, !frame->script()->anyPageIsProcessingUserGesture(), false);
return jsUndefined();
diff --git a/WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp b/WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp
index 8a92e0a..a4a7f9a 100644
--- a/WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp
+++ b/WebCore/bindings/v8/custom/V8DOMWindowCustom.cpp
@@ -65,6 +65,27 @@ v8::Handle<v8::Value> V8Custom::WindowSetTimeoutImpl(const v8::Arguments& args,
if (argumentCount < 1)
return v8::Undefined();
+ v8::Handle<v8::Value> function = args[0];
+
+ WebCore::String functionString;
+ if (!function->IsFunction())
+ functionString = function->IsString() ?
+ toWebCoreString(function) : toWebCoreString(function->ToString());
+
+ // Bail out if string conversion failed.
+ if (functionString.IsEmpty())
+ return v8::Undefined();
+
+ // Don't allow setting timeouts to run empty functions!
+ // (Bug 1009597)
+ if (functionString.length() == 0)
+ return v8::Undefined();
+ }
+
+ int32_t timeout = 0;
+ if (argumentCount >= 2)
+ timeout = args[1]->Int32Value();
+
DOMWindow* imp = V8DOMWrapper::convertToNativeObject<DOMWindow>(V8ClassIndex::DOMWINDOW, args.Holder());
if (!V8Proxy::canAccessFrame(imp->frame(), true))
@@ -75,12 +96,6 @@ v8::Handle<v8::Value> V8Custom::WindowSetTimeoutImpl(const v8::Arguments& args,
if (!scriptContext)
return v8::Undefined();
- v8::Handle<v8::Value> function = args[0];
-
- int32_t timeout = 0;
- if (argumentCount >= 2)
- timeout = args[1]->Int32Value();
-
int id;
if (function->IsFunction()) {
int paramCount = argumentCount >= 2 ? argumentCount - 2 : 0;
@@ -99,20 +114,6 @@ v8::Handle<v8::Value> V8Custom::WindowSetTimeoutImpl(const v8::Arguments& args,
id = DOMTimer::install(scriptContext, action, timeout, singleShot);
} else {
- if (!function->IsString()) {
- function = function->ToString();
- // Bail out if string conversion failed.
- if (function.IsEmpty())
- return v8::Undefined();
- }
-
- WebCore::String functionString = toWebCoreString(function);
-
- // Don't allow setting timeouts to run empty functions!
- // (Bug 1009597)
- if (functionString.length() == 0)
- return v8::Undefined();
-
id = DOMTimer::install(scriptContext, new ScheduledAction(V8Proxy::context(imp->frame()), functionString), timeout, singleShot);
}
@@ -227,6 +228,10 @@ ACCESSOR_SETTER(DOMWindowOpener)
CALLBACK_FUNC_DECL(DOMWindowAddEventListener)
{
INC_STATS("DOM.DOMWindow.addEventListener()");
+
+ String eventType = toWebCoreString(args[0]);
+ bool useCapture = args[2]->BooleanValue();
+
DOMWindow* imp = V8DOMWrapper::convertToNativeObject<DOMWindow>(V8ClassIndex::DOMWINDOW, args.Holder());
if (!V8Proxy::canAccessFrame(imp->frame(), true))
@@ -244,11 +249,8 @@ CALLBACK_FUNC_DECL(DOMWindowAddEventListener)
RefPtr<EventListener> listener = proxy->eventListeners()->findOrCreateWrapper<V8EventListener>(proxy->frame(), args[1], false);
- if (listener) {
- String eventType = toWebCoreString(args[0]);
- bool useCapture = args[2]->BooleanValue();
+ if (listener)
imp->addEventListener(eventType, listener, useCapture);
- }
return v8::Undefined();
}
@@ -257,6 +259,10 @@ CALLBACK_FUNC_DECL(DOMWindowAddEventListener)
CALLBACK_FUNC_DECL(DOMWindowRemoveEventListener)
{
INC_STATS("DOM.DOMWindow.removeEventListener()");
+
+ String eventType = toWebCoreString(args[0]);
+ bool useCapture = args[2]->BooleanValue();
+
DOMWindow* imp = V8DOMWrapper::convertToNativeObject<DOMWindow>(V8ClassIndex::DOMWINDOW, args.Holder());
if (!V8Proxy::canAccessFrame(imp->frame(), true))
@@ -273,11 +279,8 @@ CALLBACK_FUNC_DECL(DOMWindowRemoveEventListener)
RefPtr<EventListener> listener = proxy->eventListeners()->findWrapper(args[1], false);
- if (listener) {
- String eventType = toWebCoreString(args[0]);
- bool useCapture = args[2]->BooleanValue();
+ if (listener)
imp->removeEventListener(eventType, listener.get(), useCapture);
- }
return v8::Undefined();
}
@@ -318,6 +321,11 @@ CALLBACK_FUNC_DECL(DOMWindowPostMessage)
CALLBACK_FUNC_DECL(DOMWindowAtob)
{
INC_STATS("DOM.DOMWindow.atob()");
+
+ if (args[0]->IsNull())
+ return v8String("");
+ String str = toWebCoreString(args[0]);
+
DOMWindow* imp = V8DOMWrapper::convertToNativeObject<DOMWindow>(V8ClassIndex::DOMWINDOW, args.Holder());
if (!V8Proxy::canAccessFrame(imp->frame(), true))
@@ -326,16 +334,17 @@ CALLBACK_FUNC_DECL(DOMWindowAtob)
if (args.Length() < 1)
return throwError("Not enough arguments", V8Proxy::SyntaxError);
- if (args[0]->IsNull())
- return v8String("");
-
- String str = toWebCoreString(args[0]);
return convertBase64(str, false);
}
CALLBACK_FUNC_DECL(DOMWindowBtoa)
{
INC_STATS("DOM.DOMWindow.btoa()");
+
+ if (args[0]->IsNull())
+ return v8String("");
+ String str = toWebCoreString(args[0]);
+
DOMWindow* imp = V8DOMWrapper::convertToNativeObject<DOMWindow>(V8ClassIndex::DOMWINDOW, args.Holder());
if (!V8Proxy::canAccessFrame(imp->frame(), true))
@@ -344,10 +353,6 @@ CALLBACK_FUNC_DECL(DOMWindowBtoa)
if (args.Length() < 1)
return throwError("Not enough arguments", V8Proxy::SyntaxError);
- if (args[0]->IsNull())
- return v8String("");
-
- String str = toWebCoreString(args[0]);
return convertBase64(str, true);
}
@@ -563,6 +568,11 @@ static Frame* createWindow(Frame* callingFrame,
CALLBACK_FUNC_DECL(DOMWindowShowModalDialog)
{
INC_STATS("DOM.DOMWindow.showModalDialog()");
+
+ String url = toWebCoreStringWithNullOrUndefinedCheck(args[0]);
+ v8::Local<v8::Value> dialogArgs = args[1];
+ String featureArgs = toWebCoreStringWithNullOrUndefinedCheck(args[2]);
+
DOMWindow* window = V8DOMWrapper::convertToNativeObject<DOMWindow>(
V8ClassIndex::DOMWINDOW, args.Holder());
Frame* frame = window->frame();
@@ -581,10 +591,6 @@ CALLBACK_FUNC_DECL(DOMWindowShowModalDialog)
if (!canShowModalDialogNow(frame) || !allowPopUp())
return v8::Undefined();
- String url = toWebCoreStringWithNullOrUndefinedCheck(args[0]);
- v8::Local<v8::Value> dialogArgs = args[1];
- String featureArgs = toWebCoreStringWithNullOrUndefinedCheck(args[2]);
-
const HashMap<String, String> features = parseModalDialogFeatures(featureArgs);
const bool trusted = false;
@@ -652,6 +658,10 @@ CALLBACK_FUNC_DECL(DOMWindowShowModalDialog)
CALLBACK_FUNC_DECL(DOMWindowOpen)
{
INC_STATS("DOM.DOMWindow.open()");
+
+ String urlString = toWebCoreStringWithNullOrUndefinedCheck(args[0]);
+ AtomicString frameName = (args[1]->IsUndefined() || args[1]->IsNull()) ? "_blank" : AtomicString(toWebCoreString(args[1]));
+
DOMWindow* parent = V8DOMWrapper::convertToNativeObject<DOMWindow>(V8ClassIndex::DOMWINDOW, args.Holder());
Frame* frame = parent->frame();
@@ -670,9 +680,6 @@ CALLBACK_FUNC_DECL(DOMWindowOpen)
if (!page)
return v8::Undefined();
- String urlString = toWebCoreStringWithNullOrUndefinedCheck(args[0]);
- AtomicString frameName = (args[1]->IsUndefined() || args[1]->IsNull()) ? "_blank" : AtomicString(toWebCoreString(args[1]));
-
// Because FrameTree::find() returns true for empty strings, we must check
// for empty framenames. Otherwise, illegitimate window.open() calls with
// no name will pass right through the popup blocker.
@@ -848,13 +855,13 @@ void V8Custom::WindowSetLocation(DOMWindow* window, const String& relativeURL)
if (!frame)
return;
- if (!shouldAllowNavigation(frame))
- return;
-
KURL url = completeURL(relativeURL);
if (url.isNull())
return;
+ if (!shouldAllowNavigation(frame))
+ return;
+
navigateIfAllowed(frame, url, false, false);
}
@@ -875,6 +882,8 @@ CALLBACK_FUNC_DECL(DOMWindowSetInterval)
void V8Custom::ClearTimeoutImpl(const v8::Arguments& args)
{
+ int handle = toInt32(args[0]);
+
v8::Handle<v8::Object> holder = args.Holder();
DOMWindow* imp = V8DOMWrapper::convertToNativeObject<DOMWindow>(V8ClassIndex::DOMWINDOW, holder);
if (!V8Proxy::canAccessFrame(imp->frame(), true))
@@ -882,7 +891,6 @@ void V8Custom::ClearTimeoutImpl(const v8::Arguments& args)
ScriptExecutionContext* context = static_cast<ScriptExecutionContext*>(imp->document());
if (!context)
return;
- int handle = toInt32(args[0]);
DOMTimer::removeById(context, handle);
}
diff --git a/WebCore/bindings/v8/custom/V8LocationCustom.cpp b/WebCore/bindings/v8/custom/V8LocationCustom.cpp
index 3f3ff6b..17a5e7c 100644
--- a/WebCore/bindings/v8/custom/V8LocationCustom.cpp
+++ b/WebCore/bindings/v8/custom/V8LocationCustom.cpp
@@ -128,13 +128,13 @@ ACCESSOR_SETTER(LocationHref)
if (!frame)
return;
- if (!shouldAllowNavigation(frame))
- return;
-
KURL url = completeURL(toWebCoreString(value));
if (url.isNull())
return;
+ if (!shouldAllowNavigation(frame))
+ return;
+
navigateIfAllowed(frame, url, false, false);
}
@@ -288,13 +288,13 @@ CALLBACK_FUNC_DECL(LocationReplace)
if (!frame)
return v8::Undefined();
- if (!shouldAllowNavigation(frame))
- return v8::Undefined();
-
KURL url = completeURL(toWebCoreString(args[0]));
if (url.isNull())
return v8::Undefined();
+ if (!shouldAllowNavigation(frame))
+ return v8::Undefined();
+
navigateIfAllowed(frame, url, true, true);
return v8::Undefined();
}
@@ -309,13 +309,13 @@ CALLBACK_FUNC_DECL(LocationAssign)
if (!frame)
return v8::Undefined();
- if (!shouldAllowNavigation(frame))
- return v8::Undefined();
-
KURL url = completeURL(toWebCoreString(args[0]));
if (url.isNull())
return v8::Undefined();
+ if (!shouldAllowNavigation(frame))
+ return v8::Undefined();
+
navigateIfAllowed(frame, url, false, false);
return v8::Undefined();
}
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list