1034157 - [CSP] new csp implementation doesn't always send violation reports

Status: RESOLVED FIXED

(Core :: DOM: Security, defect)

Description

[Sid Stamm [:geekboy or :sstamm]]

I noticed while updating content/base/test/unit/test_cspreports.js to use the new backend that the violation reporting doesn't always work (test fails due to nsIContentPolicy disallowing the load).  Turns out we were not passing the protected document URI (mSelfURI) when checking if we can send the reports, so it was always saying no.

For example:
http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsCSPContext.cpp#186
186         this->AsyncReportViolation(aContentLocation,
187                                    aRequestOrigin,
188                                    violatedDirective,
189                                    p,             /* policy index        */
190                                    EmptyString(), /* no observer subject */
191                                    EmptyString(), /* no source file      */
192                                    EmptyString(), /* no script sample    */
193                                    0);            /* no line number      */

aRequestOrigin might not be right, since CSP should always use the "self" URI (mSelfURI) since that's the protected document corresponding to "this".

Same here:
http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsCSPContext.cpp#444

444       this->AsyncReportViolation(selfISupports, nullptr, violatedDirective, p, \
445                                  NS_LITERAL_STRING(observerTopic),             \
446                                  aSourceFile, aScriptSample, aLineNum);        \

The second arg should not be nullptr, should be the self URI like it is for frame ancestors:
http://mxr.mozilla.org/mozilla-central/source/content/base/src/nsCSPContext.cpp#1072

Comment 1

[Sid Stamm [:geekboy or :sstamm]]

Attachment: fix-violation-reporting

Comment 2

[Christoph Kerschbaumer [:ckerschb]]

Comment on attachment 8450361 [details] [diff] [review]
fix-violation-reporting

Review of attachment 8450361 [details] [diff] [review]:
-----------------------------------------------------------------

I think you got it all - good catch, also updating the 'line-number' and the CSPCONTEXTLOG goes nicely with this patch!
Let's roll this one out!

Comment 3

[Sid Stamm [:geekboy or :sstamm]]

Attachment: fix-violation-reporting

d'oh, forgot to qrefresh.  Here's the right patch.

Comment 4

[Christoph Kerschbaumer [:ckerschb]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a6a8770eb49d

Comment 5

[Christoph Kerschbaumer [:ckerschb]]

Comment on attachment 8450371 [details] [diff] [review]
fix-violation-reporting

you forgot to carry over my r+ from earlier.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a6a8770eb49d