Bug 54551 - disable execution of inline scripts when a content security policy is present
Summary: disable execution of inline scripts when a content security policy is present
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: jochen
URL:
Keywords:
Depends on:
Blocks: 53572
  Show dependency treegraph
 
Reported: 2011-02-16 06:39 PST by jochen
Modified: 2011-05-11 01:33 PDT (History)
4 users (show)

See Also:


Attachments
Patch (10.46 KB, patch)
2011-02-16 06:40 PST, jochen
abarth: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description jochen 2011-02-16 06:39:35 PST
disable execution of inline scripts when a content security policy is present
Comment 1 jochen 2011-02-16 06:40:06 PST
Created attachment 82626 [details]
Patch
Comment 2 jochen 2011-02-16 06:42:04 PST
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)
Comment 3 Early Warning System Bot 2011-02-16 06:53:32 PST
Attachment 82626 [details] did not build on qt:
Build output: http://queues.webkit.org/results/7917359
Comment 4 jochen 2011-02-16 07:13:57 PST
seems like qt is at an old revision?
Comment 5 Csaba Osztrogonác 2011-02-16 09:34:12 PST
(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?
Comment 6 Adam Barth 2011-02-16 10:13:36 PST
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.
Comment 7 Adam Barth 2011-05-11 01:33:31 PDT
This got fixed in another bug.