Bug 54551

Summary: disable execution of inline scripts when a content security policy is present
Product: WebKit Reporter: jochen
Component: New BugsAssignee: jochen
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, eric, ossy, webkit-ews
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Other   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 53572    
Attachments:
Description Flags
Patch abarth: review-

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.