Bug 54551 – disable execution of inline scripts when a content security policy is present

• Component: New Bugs
• Status: RESOLVED FIXED

disable execution of inline scripts when a content security policy is present

Created attachment 82626 [details]
Patch

i'm starting to implement the cases of javascript that shouldn't get executed: here inline scripts (will make sure script tags created by scripts aren't executed either in a separate cl)

[Early Warning System Bot]

Attachment 82626 [details] did not build on qt:
Build output: http://queues.webkit.org/results/7917359

seems like qt is at an old revision?

[Csaba Osztrogonác]

(In reply to comment #4)
> seems like qt is at an old revision?

Hmmmm .... Very strange ... Qt build works for me on trunk (r78709)
AFAIK EWS always updates before testing a patch. I have no idea
what caused this false alarm. Eric, any idea?

[Adam Barth]

Comment on attachment 82626 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=82626&action=review

> Source/WebCore/html/parser/HTMLDocumentParser.cpp:468
> +bool HTMLDocumentParser::shouldRunInlineScripts()

I see that you've patterned this after shouldLoadExternalScriptFromSrc, but shouldLoadExternalScriptFromSrc is very wrong.  It's nowhere need near the only way to run external scripts.

> Source/WebCore/html/parser/HTMLDocumentParser.h:110
> +    virtual bool shouldRunInlineScripts();

This should be a method on ContentSecurityPolicy.  ContentSecurityPolicy should encapsulate the semantics of the policy.  I'd call it something like ContentSecurityPolicy::allowInlineScripts()

> Source/WebCore/html/parser/HTMLScriptRunner.cpp:307
> +        } else if (m_host->shouldRunInlineScripts()) {

This is the wrong layer.  We should do this work in ScriptController.  We probably need to teach script controller some things it doesn't know yet.  It's probably easier to do JavaScript URLs first because ScriptController has a pretty good handle on whether it's trying to execute a JavaScript URL.

> Source/WebCore/page/ContentSecurityPolicy.h:42
> +    bool hasPolicy() const { return m_havePolicy; }

This should be private.

[Adam Barth]

This got fixed in another bug.