Skip to content

Commit

Permalink
Bug 1418241 - CSP violation: blockedURI inline/eval, r=ckerschb
Browse files Browse the repository at this point in the history
  • Loading branch information
bakulf committed Jul 17, 2018
1 parent 357119b commit 35496d6
Show file tree
Hide file tree
Showing 25 changed files with 29 additions and 112 deletions.
31 changes: 22 additions & 9 deletions dom/security/nsCSPContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,7 @@ nsCSPContext::reportInlineViolation(nsContentPolicyType aContentType,

nsCOMPtr<nsISupportsCString> selfICString(do_CreateInstance(NS_SUPPORTS_CSTRING_CONTRACTID));
if (selfICString) {
selfICString->SetData(nsDependentCString("self"));
selfICString->SetData(nsDependentCString("inline"));
}
nsCOMPtr<nsISupports> selfISupports(do_QueryInterface(selfICString));

Expand Down Expand Up @@ -672,7 +672,16 @@ nsCSPContext::LogViolationDetails(uint16_t aViolationType,

nsCOMPtr<nsISupportsCString> selfICString(do_CreateInstance(NS_SUPPORTS_CSTRING_CONTRACTID));
if (selfICString) {
selfICString->SetData(nsDependentCString("self"));
if (aViolationType == nsIContentSecurityPolicy::VIOLATION_TYPE_EVAL) {
selfICString->SetData(nsDependentCString("eval"));
} else if (aViolationType == nsIContentSecurityPolicy::VIOLATION_TYPE_INLINE_SCRIPT ||
aViolationType == nsIContentSecurityPolicy::VIOLATION_TYPE_INLINE_STYLE) {
selfICString->SetData(nsDependentCString("inline"));
} else {
// All the other types should have a URL, but just in case, let's use
// 'self' here.
selfICString->SetData(nsDependentCString("self"));
}
}
nsCOMPtr<nsISupports> selfISupports(do_QueryInterface(selfICString));

Expand Down Expand Up @@ -881,6 +890,7 @@ StripURIForReporting(nsIURI* aURI,
nsresult
nsCSPContext::GatherSecurityPolicyViolationEventData(
nsIURI* aBlockedURI,
const nsACString& aBlockedString,
nsIURI* aOriginalURI,
nsAString& aViolatedDirective,
uint32_t aViolatedPolicyIndex,
Expand Down Expand Up @@ -909,6 +919,8 @@ nsCSPContext::GatherSecurityPolicyViolationEventData(
nsAutoCString reportBlockedURI;
StripURIForReporting(aBlockedURI, mSelfURI, reportBlockedURI);
aViolationEventInit.mBlockedURI = NS_ConvertUTF8toUTF16(reportBlockedURI);
} else {
aViolationEventInit.mBlockedURI = NS_ConvertUTF8toUTF16(aBlockedString);
}

// effective-directive
Expand Down Expand Up @@ -1289,8 +1301,15 @@ class CSPReportSenderRunnable final : public Runnable
mozilla::dom::SecurityPolicyViolationEventInit init;
// mBlockedContentSource could be a URI or a string.
nsCOMPtr<nsIURI> blockedURI = do_QueryInterface(mBlockedContentSource);
// if mBlockedContentSource is not a URI, it could be a string
nsCOMPtr<nsISupportsCString> blockedICString = do_QueryInterface(mBlockedContentSource);
nsAutoCString blockedDataStr;
if (blockedICString) {
blockedICString->GetData(blockedDataStr);
}

rv = mCSPContext->GatherSecurityPolicyViolationEventData(
blockedURI, mOriginalURI,
blockedURI, blockedDataStr, mOriginalURI,
mViolatedDirective, mViolatedPolicyIndex,
mSourceFile, mScriptSample, mLineNum, mColumnNum,
init);
Expand All @@ -1308,10 +1327,6 @@ class CSPReportSenderRunnable final : public Runnable
mCSPContext->SendReports(init, mViolatedPolicyIndex);

// 3) log to console (one per policy violation)
// if mBlockedContentSource is not a URI, it could be a string
nsCOMPtr<nsISupportsCString> blockedString = do_QueryInterface(mBlockedContentSource);

nsCString blockedDataStr;

if (blockedURI) {
blockedURI->GetSpec(blockedDataStr);
Expand All @@ -1324,8 +1339,6 @@ class CSPReportSenderRunnable final : public Runnable
blockedDataStr.Append(NS_ConvertUTF16toUTF8(nsContentUtils::GetLocalizedEllipsis()));
}
}
} else if (blockedString) {
blockedString->GetData(blockedDataStr);
}

if (blockedDataStr.Length() > 0) {
Expand Down
1 change: 1 addition & 0 deletions dom/security/nsCSPContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ class nsCSPContext : public nsIContentSecurityPolicy
*/
nsresult GatherSecurityPolicyViolationEventData(
nsIURI* aBlockedURI,
const nsACString& aBlockedString,
nsIURI* aOriginalURI,
nsAString& aViolatedDirective,
uint32_t aViolatedPolicyIndex,
Expand Down
2 changes: 1 addition & 1 deletion dom/security/test/csp/test_report.html
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
ok(cspReport["referrer"].startsWith("http://mochi.test:8888/tests/dom/security/test/csp/test_report.html"),
"Incorrect referrer");

is(cspReport["blocked-uri"], "", "Incorrect blocked-uri");
is(cspReport["blocked-uri"], "inline", "Incorrect blocked-uri");

is(cspReport["violated-directive"], "default-src", "Incorrect violated-directive");

Expand Down
8 changes: 4 additions & 4 deletions dom/security/test/unit/test_csp_reports.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ function run_test() {
"/foo/self");

// test that inline script violations cause a report.
makeTest(0, {"blocked-uri": ""}, false,
makeTest(0, {"blocked-uri": "inline"}, false,
function(csp) {
let inlineOK = true;
inlineOK = csp.getAllowsInline(Ci.nsIContentPolicy.TYPE_SCRIPT,
Expand All @@ -119,7 +119,7 @@ function run_test() {
});

// test that eval violations cause a report.
makeTest(1, {"blocked-uri": "",
makeTest(1, {"blocked-uri": "eval",
// JSON script-sample is UTF8 encoded
"script-sample" : "\xc2\xa3\xc2\xa5\xc2\xb5\xe5\x8c\x97\xf0\xa0\x9d\xb9",
"line-number": 1,
Expand Down Expand Up @@ -156,7 +156,7 @@ function run_test() {
});

// test that inline script violations cause a report in report-only policy
makeTest(3, {"blocked-uri": ""}, true,
makeTest(3, {"blocked-uri": "inline"}, true,
function(csp) {
let inlineOK = true;
inlineOK = csp.getAllowsInline(Ci.nsIContentPolicy.TYPE_SCRIPT,
Expand All @@ -172,7 +172,7 @@ function run_test() {
});

// test that eval violations cause a report in report-only policy
makeTest(4, {"blocked-uri": ""}, true,
makeTest(4, {"blocked-uri": "inline"}, true,
function(csp) {
let evalOK = true, oReportViolation = {'value': false};
evalOK = csp.getAllowsEval(oReportViolation);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,3 @@
expected: TIMEOUT
[Direct block, cross-origin = full URL in report]
expected: TIMEOUT

Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
[report-strips-fragment.html]
disabled:
if verify: fails in verify mode
expected: TIMEOUT
[Reported document URI does not contain fragments.]
expected: TIMEOUT

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,3 @@
expected: TIMEOUT
[Non-whitelisted script injected via `appendChild` is not allowed with `strict-dynamic` + a nonce+whitelist double policy.]
expected: TIMEOUT

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,14 +1,5 @@
[script-sample-no-opt-in.html]
expected: TIMEOUT
[Inline script should not have a sample.]
expected: FAIL

[Inline event handlers should not have a sample.]
expected: FAIL

[JavaScript URLs in iframes should not have a sample.]
expected: TIMEOUT

[eval()-alikes should not have a sample.]
expected: TIMEOUT

Original file line number Diff line number Diff line change
@@ -1,19 +1,4 @@
[script-sample.html]
expected: TIMEOUT
[Inline script should have a sample.]
expected: FAIL

[Inline event handlers should have a sample.]
expected: FAIL

[JavaScript URLs in iframes should have a sample.]
expected: TIMEOUT

[eval() should have a sample.]
expected: TIMEOUT

[setInterval() should have a sample.]
expected: TIMEOUT

[setTimeout() should have a sample.]
expected: TIMEOUT

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
[targeting.html]
prefs: [dom.webcomponents.shadowdom.enabled:true]
expected: TIMEOUT
[These tests should not fail.]
expected: NOTRUN

[Inline violations target the right element.]
expected: FAIL

[Correct targeting inside shadow tree (inline handler).]
disabled: https://bugzilla.mozilla.org/show_bug.cgi?id=1404842

Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -835,7 +835,7 @@ function awaitCSP(urlsPromise) {
let report = body["csp-report"];

let origURL = report["blocked-uri"];
if (origURL !== "self" && origURL !== "") {
if (origURL !== "inline" && origURL !== "") {
let {baseURL} = getOriginBase(origURL);

if (expectedURLs.has(baseURL)) {
Expand Down

0 comments on commit 35496d6

Please sign in to comment.