Bug 85553 - CSP: Eval isn't blocked in about:blank subframes
: CSP: Eval isn't blocked in about:blank subframes
Status: RESOLVED FIXED
: WebKit
New Bugs
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
:
:
:
: 53572
  Show dependency treegraph
 
Reported: 2012-05-03 16:16 PST by
Modified: 2012-05-04 11:37 PST (History)


Attachments
Patch (4.81 KB, patch)
2012-05-03 16:19 PST, Adam Barth
no flags Review Patch | Details | Formatted Diff | Diff
Patch (13.25 KB, patch)
2012-05-03 18:45 PST, Adam Barth
no flags Review Patch | Details | Formatted Diff | Diff
Fix race condition in test (1.55 KB, patch)
2012-05-04 10:52 PST, Adam Barth
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2012-05-03 16:16:11 PST
ContentSecurityOrigin shouldn't be RefCounted
------- Comment #1 From 2012-05-03 16:19:44 PST -------
Created an attachment (id=140123) [details]
Patch
------- Comment #2 From 2012-05-03 16:23:53 PST -------
(From update of attachment 140123 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=140123&action=review

> Source/WebCore/ChangeLog:15
> +        I don't think this has any observable effects, so I'm not sure how to
> +        write a test for it.

You should explain better what the backpointer is used for and what the presumable effects of this change are, even if you don't believe they're necessarily observable or easily testable.
------- Comment #3 From 2012-05-03 16:28:40 PST -------
    if (!m_scriptExecutionContext->isDocument())
        return;

    m_scriptExecutionContext->addConsoleMessage(JSMessageSource, LogMessageType, ErrorMessageLevel, message);

        PingLoader::reportContentSecurityPolicyViolation(frame, m_reportURLs[i], report);

(It's also used a bunch when parsing/enforcing a policy, but that all happens before it gets shared.)

I think the worst thing that can happen is that the console message shows up in the console fro the wrong frame or that the PingLoader uses the wrong frame as the context for the report network request (e.g., maybe the referrer would end up wrong on that request).
------- Comment #4 From 2012-05-03 16:35:14 PST -------
(From update of attachment 140123 [details])
I think I figured out how to test it.  I don't think the sandbox directive is properly applied to the subframe...  Now I need to figure out how to make that into a test.
------- Comment #5 From 2012-05-03 16:36:01 PST -------
Actually, that isn't testable either because sandbox automatically inherits into subframes!  Hum...
------- Comment #6 From 2012-05-03 17:47:09 PST -------
I figured out a way to test it, because I'm amazing.
------- Comment #7 From 2012-05-03 18:45:51 PST -------
Created an attachment (id=140143) [details]
Patch
------- Comment #8 From 2012-05-03 19:04:42 PST -------
(From update of attachment 140143 [details])
Woohoo!
------- Comment #9 From 2012-05-03 20:44:24 PST -------
(From update of attachment 140143 [details])
Clearing flags on attachment: 140143

Committed r116066: <http://trac.webkit.org/changeset/116066>
------- Comment #10 From 2012-05-03 20:44:28 PST -------
All reviewed patches have been landed.  Closing bug.
------- Comment #11 From 2012-05-04 00:04:39 PST -------
I'm marking eval-blocked-in-about-blank-iframe.html as SLOW in debug based on this:

http://test-results.appspot.com/dashboards/flakiness_dashboard.html#tests=http%2Ftests%2Fsecurity%2FcontentSecurityPolicy%2Feval-blocked-in-about-blank-iframe.html
------- Comment #12 From 2012-05-04 10:11:58 PST -------
I think the test just has a bug.  It shouldn't be slow.
------- Comment #13 From 2012-05-04 10:15:52 PST -------
(In reply to comment #12)
> I think the test just has a bug.  It shouldn't be slow.

yup -- this actually happened to be timeout, not just slow (I already updated the expectations)
------- Comment #14 From 2012-05-04 10:21:41 PST -------
I think it's a race.  Will fix shortly.
------- Comment #15 From 2012-05-04 10:52:12 PST -------
Created an attachment (id=140274) [details]
Fix race condition in test
------- Comment #16 From 2012-05-04 11:00:58 PST -------
(From update of attachment 140274 [details])
LGTM.
------- Comment #17 From 2012-05-04 11:37:20 PST -------
(From update of attachment 140274 [details])
Clearing flags on attachment: 140274

Committed r116132: <http://trac.webkit.org/changeset/116132>
------- Comment #18 From 2012-05-04 11:37:25 PST -------
All reviewed patches have been landed.  Closing bug.