JSC should throw a more descriptive exception when blocking 'eval' via CSP.

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

Patch by Mike West <mkwst@chromium.org> on 2012-09-14
Reviewed by Geoffrey Garen.

Source/JavaScriptCore:

Unless explicitly whitelisted, the 'script-src' Content Security Policy
directive blocks 'eval' and 'eval'-like constructs such as
'new Function()'. When 'eval' is encountered in code, an 'EvalError' is
thrown, but the associated message is poor: "Eval is disabled" doesn't
give developers enough information about why their code isn't behaving
as expected.

This patch adds an 'errorMessage' parameter to the JavaScriptCore method
used to disable 'eval'; ContentSecurityPolicy has the opportunity to
pass in a more detailed and descriptive error that contains more context
for the developer.

* runtime/Executable.cpp:
(JSC::EvalExecutable::compileInternal):
    Drop the hard-coded "Eval is disabled" error message in favor of
    reading the error message off the global object.
* runtime/FunctionConstructor.cpp:
(JSC::FunctionConstructor::getCallData):
    Drop the hard-coded "Function constructor is disabled" error message
    in favor of reading the error message off the global object.
* runtime/JSGlobalObject.h:
(JSGlobalObject):
(JSC::JSGlobalObject::evalEnabled):
    Making this accessor method const.
(JSC::JSGlobalObject::evalDisabledErrorMessage):
    Accessor for the error message set via 'setEvalDisabled'.
(JSC::JSGlobalObject::setEvalEnabled):
    Adding an 'errorMessage' parameter which is stored on the global
    object, and used when exceptions are thrown.

Source/WebCore:

Unless explicitly whitelisted, the 'script-src' Content Security Policy
directive blocks 'eval' and 'eval'-like constructs such as
'new Function()'. When 'eval' is encountered in code, an 'EvalError' is
thrown, but the associated message is poor: "Eval is disabled" doesn't
give developers enough information about why their code isn't behaving
as expected.

This patch adds an 'errorMessage' parameter to the JavaScriptCore method
used to disable 'eval'; ContentSecurityPolicy has the opportunity to
pass in a more detailed and descriptive error that contains more context
for the developer.

The new error message is tested by adjusting existing tests; nothing new
is required.

* bindings/js/ScriptController.cpp:
(WebCore::ScriptController::initScript):
    Read the error message off the document's ContentSecurityPolicy.
(WebCore::ScriptController::disableEval):
* bindings/js/ScriptController.h:
(ScriptController):
    Pipe the error message through to JSGlobalObject when disabling eval
* bindings/js/WorkerScriptController.cpp:
(WebCore::WorkerScriptController::disableEval):
* bindings/js/WorkerScriptController.h:
(WorkerScriptController):
    Pipe the error message through to JSGlobalObject when disabling eval
* bindings/v8/ScriptController.cpp:
(WebCore::ScriptController::disableEval):
* bindings/v8/ScriptController.h:
(ScriptController):
* bindings/v8/WorkerScriptController.cpp:
(WebCore::WorkerScriptController::disableEval):
* bindings/v8/WorkerScriptController.h:
(WorkerScriptController):
    Placeholder for V8 piping to be built in webk.it/94332.
* dom/Document.cpp:
(WebCore::Document::disableEval):
* dom/Document.h:
(Document):
* dom/ScriptExecutionContext.h:
(ScriptExecutionContext):
    Pipe the error message through to the ScriptController when
    disabling eval.
* page/ContentSecurityPolicy.cpp:
(WebCore::CSPDirectiveList::evalDisabledErrorMessage):
    Accessor for the error message that ought be displayed to developers
    when 'eval' used while disabled for a specific directive list.
(WebCore::CSPDirectiveList::setEvalDisabledErrorMessage):
    Mutator for the error message that ought be displayed to developers
    when 'eval' used while disabled for a specific directive list.
(CSPDirectiveList):
(WebCore::CSPDirectiveList::create):
    Upon creation of a CSPDirectiveList, set the error message if the
    directive list disables 'eval'.
(WebCore::ContentSecurityPolicy::didReceiveHeader):
    Pass the error message into ScriptExecutionContext::disableEval.
(WebCore::ContentSecurityPolicy::evalDisabledErrorMessage):
    Public accessor for the policy's error message; walks the list of
    directive lists and returns the first error message found.
(WebCore):
* page/ContentSecurityPolicy.h:
* workers/WorkerContext.cpp:
(WebCore::WorkerContext::disableEval):
* workers/WorkerContext.h:
(WorkerContext):
    Pipe the error message through to the ScriptController when
    disabling eval.

LayoutTests:

* http/tests/security/contentSecurityPolicy/eval-blocked-expected.txt:
* http/tests/security/contentSecurityPolicy/eval-blocked-in-about-blank-iframe-expected.txt:
* http/tests/security/contentSecurityPolicy/function-constructor-blocked-expected.txt:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@128670 268f45cc-cd09-0410-ab3c-d52691b4dbfc
parent 6e720dd4
2012-09-14 Mike West <mkwst@chromium.org>
JSC should throw a more descriptive exception when blocking 'eval' via CSP.
https://bugs.webkit.org/show_bug.cgi?id=94331
Reviewed by Geoffrey Garen.
* http/tests/security/contentSecurityPolicy/eval-blocked-expected.txt:
* http/tests/security/contentSecurityPolicy/eval-blocked-in-about-blank-iframe-expected.txt:
* http/tests/security/contentSecurityPolicy/function-constructor-blocked-expected.txt:
2012-09-14 Ojan Vafai <ojan@chromium.org>
Mark compositing/geometry/fixed-position-transform-composited-page-scale.html flaky
CONSOLE MESSAGE: line 12: EvalError: Eval is disabled
CONSOLE MESSAGE: line 15: EvalError: Eval is disabled
CONSOLE MESSAGE: line 12: EvalError: Refused to evaluate a string as JavaScript because 'unsafe-eval' is not an allowed source of script in the following Content Security Policy directive: "script-src 'unsafe-inline'".
CONSOLE MESSAGE: line 15: EvalError: Refused to evaluate a string as JavaScript because 'unsafe-eval' is not an allowed source of script in the following Content Security Policy directive: "script-src 'unsafe-inline'".
ALERT: /PASS/
CONSOLE MESSAGE: line 1: EvalError: Eval is disabled
CONSOLE MESSAGE: line 1: EvalError: Refused to evaluate a string as JavaScript because 'unsafe-eval' is not an allowed source of script in the following Content Security Policy directive: "script-src 'unsafe-inline'".
Eval should be blocked in the iframe, but inline script should be allowed.
CONSOLE MESSAGE: line 12: EvalError: Function constructor is disabled
CONSOLE MESSAGE: line 12: EvalError: Refused to evaluate a string as JavaScript because 'unsafe-eval' is not an allowed source of script in the following Content Security Policy directive: "script-src 'unsafe-inline'".
2012-09-14 Mike West <mkwst@chromium.org>
JSC should throw a more descriptive exception when blocking 'eval' via CSP.
https://bugs.webkit.org/show_bug.cgi?id=94331
Reviewed by Geoffrey Garen.
Unless explicitly whitelisted, the 'script-src' Content Security Policy
directive blocks 'eval' and 'eval'-like constructs such as
'new Function()'. When 'eval' is encountered in code, an 'EvalError' is
thrown, but the associated message is poor: "Eval is disabled" doesn't
give developers enough information about why their code isn't behaving
as expected.
This patch adds an 'errorMessage' parameter to the JavaScriptCore method
used to disable 'eval'; ContentSecurityPolicy has the opportunity to
pass in a more detailed and descriptive error that contains more context
for the developer.
* runtime/Executable.cpp:
(JSC::EvalExecutable::compileInternal):
Drop the hard-coded "Eval is disabled" error message in favor of
reading the error message off the global object.
* runtime/FunctionConstructor.cpp:
(JSC::FunctionConstructor::getCallData):
Drop the hard-coded "Function constructor is disabled" error message
in favor of reading the error message off the global object.
* runtime/JSGlobalObject.h:
(JSGlobalObject):
(JSC::JSGlobalObject::evalEnabled):
Making this accessor method const.
(JSC::JSGlobalObject::evalDisabledErrorMessage):
Accessor for the error message set via 'setEvalDisabled'.
(JSC::JSGlobalObject::setEvalEnabled):
Adding an 'errorMessage' parameter which is stored on the global
object, and used when exceptions are thrown.
2012-09-14 Filip Pizlo <fpizlo@apple.com>
bbc homepage crashes immediately
......
......@@ -202,7 +202,7 @@ JSObject* EvalExecutable::compileInternal(ExecState* exec, JSScope* scope, JITCo
m_evalCodeBlock = newCodeBlock.release();
} else {
if (!lexicalGlobalObject->evalEnabled())
return throwError(exec, createEvalError(exec, ASCIILiteral("Eval is disabled")));
return throwError(exec, createEvalError(exec, lexicalGlobalObject->evalDisabledErrorMessage()));
RefPtr<EvalNode> evalNode = parse<EvalNode>(globalData, lexicalGlobalObject, m_source, 0, Identifier(), isStrictMode() ? JSParseStrict : JSParseNormal, EvalNode::isFunctionNode ? JSParseFunctionCode : JSParseProgramCode, lexicalGlobalObject->debugger(), exec, &exception);
if (!evalNode) {
ASSERT(exception);
......
......@@ -82,7 +82,7 @@ CallType FunctionConstructor::getCallData(JSCell*, CallData& callData)
JSObject* constructFunction(ExecState* exec, JSGlobalObject* globalObject, const ArgList& args, const Identifier& functionName, const String& sourceURL, const TextPosition& position)
{
if (!globalObject->evalEnabled())
return throwError(exec, createEvalError(exec, ASCIILiteral("Function constructor is disabled")));
return throwError(exec, createEvalError(exec, globalObject->evalDisabledErrorMessage()));
return constructFunctionSkippingEvalEnabledCheck(exec, globalObject, args, functionName, sourceURL, position);
}
......
......@@ -155,6 +155,7 @@ namespace JSC {
WeakRandom m_weakRandom;
bool m_evalEnabled;
String m_evalDisabledErrorMessage;
bool m_experimentsEnabled;
static JS_EXPORTDATA const GlobalObjectMethodTable s_globalObjectMethodTable;
......@@ -304,8 +305,13 @@ namespace JSC {
bool isDynamicScope(bool& requiresDynamicChecks) const;
void setEvalEnabled(bool enabled) { m_evalEnabled = enabled; }
bool evalEnabled() { return m_evalEnabled; }
bool evalEnabled() const { return m_evalEnabled; }
const String& evalDisabledErrorMessage() const { return m_evalDisabledErrorMessage; }
void setEvalEnabled(bool enabled, const String& errorMessage = String())
{
m_evalEnabled = enabled;
m_evalDisabledErrorMessage = errorMessage;
}
void resetPrototype(JSGlobalData&, JSValue prototype);
......
2012-09-14 Mike West <mkwst@chromium.org>
JSC should throw a more descriptive exception when blocking 'eval' via CSP.
https://bugs.webkit.org/show_bug.cgi?id=94331
Reviewed by Geoffrey Garen.
Unless explicitly whitelisted, the 'script-src' Content Security Policy
directive blocks 'eval' and 'eval'-like constructs such as
'new Function()'. When 'eval' is encountered in code, an 'EvalError' is
thrown, but the associated message is poor: "Eval is disabled" doesn't
give developers enough information about why their code isn't behaving
as expected.
This patch adds an 'errorMessage' parameter to the JavaScriptCore method
used to disable 'eval'; ContentSecurityPolicy has the opportunity to
pass in a more detailed and descriptive error that contains more context
for the developer.
The new error message is tested by adjusting existing tests; nothing new
is required.
* bindings/js/ScriptController.cpp:
(WebCore::ScriptController::initScript):
Read the error message off the document's ContentSecurityPolicy.
(WebCore::ScriptController::disableEval):
* bindings/js/ScriptController.h:
(ScriptController):
Pipe the error message through to JSGlobalObject when disabling eval
* bindings/js/WorkerScriptController.cpp:
(WebCore::WorkerScriptController::disableEval):
* bindings/js/WorkerScriptController.h:
(WorkerScriptController):
Pipe the error message through to JSGlobalObject when disabling eval
* bindings/v8/ScriptController.cpp:
(WebCore::ScriptController::disableEval):
* bindings/v8/ScriptController.h:
(ScriptController):
* bindings/v8/WorkerScriptController.cpp:
(WebCore::WorkerScriptController::disableEval):
* bindings/v8/WorkerScriptController.h:
(WorkerScriptController):
Placeholder for V8 piping to be built in webk.it/94332.
* dom/Document.cpp:
(WebCore::Document::disableEval):
* dom/Document.h:
(Document):
* dom/ScriptExecutionContext.h:
(ScriptExecutionContext):
Pipe the error message through to the ScriptController when
disabling eval.
* page/ContentSecurityPolicy.cpp:
(WebCore::CSPDirectiveList::evalDisabledErrorMessage):
Accessor for the error message that ought be displayed to developers
when 'eval' used while disabled for a specific directive list.
(WebCore::CSPDirectiveList::setEvalDisabledErrorMessage):
Mutator for the error message that ought be displayed to developers
when 'eval' used while disabled for a specific directive list.
(CSPDirectiveList):
(WebCore::CSPDirectiveList::create):
Upon creation of a CSPDirectiveList, set the error message if the
directive list disables 'eval'.
(WebCore::ContentSecurityPolicy::didReceiveHeader):
Pass the error message into ScriptExecutionContext::disableEval.
(WebCore::ContentSecurityPolicy::evalDisabledErrorMessage):
Public accessor for the policy's error message; walks the list of
directive lists and returns the first error message found.
(WebCore):
* page/ContentSecurityPolicy.h:
* workers/WorkerContext.cpp:
(WebCore::WorkerContext::disableEval):
* workers/WorkerContext.h:
(WorkerContext):
Pipe the error message through to the ScriptController when
disabling eval.
2012-09-14 Dan Carney <dcarney@google.com>
Remove V8DOMWindowShell::getEntered
......@@ -222,7 +222,7 @@ JSDOMWindowShell* ScriptController::initScript(DOMWrapperWorld* world)
windowShell->window()->updateDocument();
if (m_frame->document())
windowShell->window()->setEvalEnabled(m_frame->document()->contentSecurityPolicy()->allowEval(0, ContentSecurityPolicy::SuppressReport));
windowShell->window()->setEvalEnabled(m_frame->document()->contentSecurityPolicy()->allowEval(0, ContentSecurityPolicy::SuppressReport), m_frame->document()->contentSecurityPolicy()->evalDisabledErrorMessage());
if (Page* page = m_frame->page()) {
attachDebugger(windowShell, page->debugger());
......@@ -250,12 +250,12 @@ void ScriptController::enableEval()
windowShell->window()->setEvalEnabled(true);
}
void ScriptController::disableEval()
void ScriptController::disableEval(const String& errorMessage)
{
JSDOMWindowShell* windowShell = existingWindowShell(mainThreadNormalWorld());
if (!windowShell)
return;
windowShell->window()->setEvalEnabled(false);
windowShell->window()->setEvalEnabled(false, errorMessage);
}
bool ScriptController::processingUserGesture()
......
......@@ -105,7 +105,7 @@ public:
WTF::TextPosition eventHandlerPosition() const;
void enableEval();
void disableEval();
void disableEval(const String& errorMessage);
static bool processingUserGesture();
......
......@@ -189,12 +189,12 @@ bool WorkerScriptController::isExecutionForbidden() const
return m_executionForbidden;
}
void WorkerScriptController::disableEval()
void WorkerScriptController::disableEval(const String& errorMessage)
{
initScriptIfNeeded();
JSLockHolder lock(globalData());
m_workerContextWrapper->setEvalEnabled(false);
m_workerContextWrapper->setEvalEnabled(false, errorMessage);
}
} // namespace WebCore
......
......@@ -75,7 +75,7 @@ namespace WebCore {
void forbidExecution();
bool isExecutionForbidden() const;
void disableEval();
void disableEval(const String& errorMessage);
JSC::JSGlobalData* globalData() { return m_globalData.get(); }
......
......@@ -487,7 +487,7 @@ void ScriptController::enableEval()
v8Context->AllowCodeGenerationFromStrings(true);
}
void ScriptController::disableEval()
void ScriptController::disableEval(const String& /* errorMessage */)
{
v8::HandleScope handleScope;
v8::Handle<v8::Context> v8Context = windowShell()->context();
......
......@@ -121,7 +121,7 @@ public:
bool haveInterpreter() const;
void enableEval();
void disableEval();
void disableEval(const String& errorMessage);
static bool canAccessFromCurrentOrigin(Frame*);
......
......@@ -132,7 +132,7 @@ bool WorkerScriptController::isExecutionForbidden() const
return m_executionForbidden;
}
void WorkerScriptController::disableEval()
void WorkerScriptController::disableEval(const String& /* errorMessage */)
{
m_proxy->setEvalAllowed(false);
}
......
......@@ -73,7 +73,7 @@ namespace WebCore {
void forbidExecution();
bool isExecutionForbidden() const;
void disableEval();
void disableEval(const String&);
// Returns WorkerScriptController for the currently executing context. 0 will be returned if the current executing context is not the worker context.
static WorkerScriptController* controllerForContext();
......
......@@ -2826,12 +2826,12 @@ String Document::userAgent(const KURL& url) const
return frame() ? frame()->loader()->userAgent(url) : String();
}
void Document::disableEval()
void Document::disableEval(const String& errorMessage)
{
if (!frame())
return;
frame()->script()->disableEval();
frame()->script()->disableEval(errorMessage);
}
bool Document::canNavigate(Frame* targetFrame)
......
......@@ -645,7 +645,7 @@ public:
virtual String userAgent(const KURL&) const;
virtual void disableEval();
virtual void disableEval(const String& errorMessage);
bool canNavigate(Frame* targetFrame);
Frame* findUnsafeParentScrollPropagationBoundary();
......
......@@ -79,7 +79,7 @@ public:
virtual String userAgent(const KURL&) const = 0;
virtual void disableEval() = 0;
virtual void disableEval(const String& errorMessage) = 0;
bool sanitizeScriptError(String& errorMessage, int& lineNumber, String& sourceURL);
void reportException(const String& errorMessage, int lineNumber, const String& sourceURL, PassRefPtr<ScriptCallStack>);
......
......@@ -726,6 +726,7 @@ public:
bool allowFormAction(const KURL&, ContentSecurityPolicy::ReportingStatus) const;
void gatherReportURIs(DOMStringList&) const;
const String& evalDisabledErrorMessage() { return m_evalDisabledErrorMessage; }
private:
explicit CSPDirectiveList(ContentSecurityPolicy*);
......@@ -751,6 +752,8 @@ private:
bool checkSource(SourceListDirective*, const KURL&) const;
bool checkMediaType(MediaListDirective*, const String& type, const String& typeAttribute) const;
void setEvalDisabledErrorMessage(const String& errorMessage) { m_evalDisabledErrorMessage = errorMessage; }
bool checkEvalAndReportViolation(SourceListDirective*, const String& consoleMessage, const String& contextURL = String(), const WTF::OrdinalNumber& contextLine = WTF::OrdinalNumber::beforeFirst(), PassRefPtr<ScriptCallStack> = 0) const;
bool checkInlineAndReportViolation(SourceListDirective*, const String& consoleMessage, const String& contextURL, const WTF::OrdinalNumber& contextLine) const;
bool checkNonceAndReportViolation(NonceDirective*, const String& nonce, const String& consoleMessage, const String& contextURL, const WTF::OrdinalNumber& contextLine) const;
......@@ -760,6 +763,7 @@ private:
bool denyIfEnforcingPolicy() const { return m_reportOnly; }
ContentSecurityPolicy* m_policy;
String m_header;
bool m_reportOnly;
......@@ -779,6 +783,8 @@ private:
OwnPtr<SourceListDirective> m_styleSrc;
Vector<KURL> m_reportURIs;
String m_evalDisabledErrorMessage;
};
CSPDirectiveList::CSPDirectiveList(ContentSecurityPolicy* policy)
......@@ -803,6 +809,11 @@ PassOwnPtr<CSPDirectiveList> CSPDirectiveList::create(ContentSecurityPolicy* pol
break;
}
if (!directives->checkEval(directives->operativeDirective(directives->m_scriptSrc.get()))) {
String message = makeString("Refused to evaluate a string as JavaScript because 'unsafe-eval' is not an allowed source of script in the following Content Security Policy directive: \"", directives->operativeDirective(directives->m_scriptSrc.get())->text(), "\".\n");
directives->setEvalDisabledErrorMessage(message);
}
return directives.release();
}
......@@ -1261,7 +1272,7 @@ void ContentSecurityPolicy::didReceiveHeader(const String& header, HeaderType ty
}
if (!allowEval(0, SuppressReport))
m_scriptExecutionContext->disableEval();
m_scriptExecutionContext->disableEval(evalDisabledErrorMessage());
}
void ContentSecurityPolicy::setOverrideAllowInlineStyle(bool value)
......@@ -1349,6 +1360,15 @@ bool ContentSecurityPolicy::allowEval(PassRefPtr<ScriptCallStack> callStack, Con
return isAllowedByAllWithCallStack<&CSPDirectiveList::allowEval>(m_policies, callStack, reportingStatus);
}
String ContentSecurityPolicy::evalDisabledErrorMessage() const
{
for (size_t i = 0; i < m_policies.size(); ++i) {
if (!m_policies[i]->allowEval(0, SuppressReport))
return m_policies[i]->evalDisabledErrorMessage();
}
return String();
}
bool ContentSecurityPolicy::allowScriptNonce(const String& nonce, const String& contextURL, const WTF::OrdinalNumber& contextLine, const KURL& url) const
{
return isAllowedByAllWithNonce<&CSPDirectiveList::allowScriptNonce>(m_policies, nonce, contextURL, contextLine, url);
......@@ -1357,7 +1377,7 @@ bool ContentSecurityPolicy::allowScriptNonce(const String& nonce, const String&
bool ContentSecurityPolicy::allowPluginType(const String& type, const String& typeAttribute, const KURL& url, ContentSecurityPolicy::ReportingStatus reportingStatus) const
{
for (size_t i = 0; i < m_policies.size(); ++i) {
if (!m_policies[i].get()->allowPluginType(type, typeAttribute, url, reportingStatus))
if (!m_policies[i]->allowPluginType(type, typeAttribute, url, reportingStatus))
return false;
}
return true;
......@@ -1416,7 +1436,7 @@ bool ContentSecurityPolicy::isActive() const
void ContentSecurityPolicy::gatherReportURIs(DOMStringList& list) const
{
for (size_t i = 0; i < m_policies.size(); ++i)
m_policies[i].get()->gatherReportURIs(list);
m_policies[i]->gatherReportURIs(list);
}
SecurityOrigin* ContentSecurityPolicy::securityOrigin() const
......
......@@ -112,6 +112,7 @@ public:
KURL completeURL(const String&) const;
SecurityOrigin* securityOrigin() const;
void enforceSandboxFlags(SandboxFlags) const;
String evalDisabledErrorMessage() const;
private:
explicit ContentSecurityPolicy(ScriptExecutionContext*);
......
......@@ -148,9 +148,9 @@ String WorkerContext::userAgent(const KURL&) const
return m_userAgent;
}
void WorkerContext::disableEval()
void WorkerContext::disableEval(const String& errorMessage)
{
m_script->disableEval();
m_script->disableEval(errorMessage);
}
WorkerLocation* WorkerContext::location() const
......
......@@ -72,7 +72,7 @@ namespace WebCore {
const GroupSettings* groupSettings() { return m_groupSettings.get(); }
virtual String userAgent(const KURL&) const;
virtual void disableEval();
virtual void disableEval(const String& errorMessage);
WorkerScriptController* script() { return m_script.get(); }
void clearScript() { m_script.clear(); }
......
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