Closed Bug 1100630 Opened 5 years ago Closed 5 months ago

Provide source file and location for CSP violations

Categories

(Core :: DOM: Security, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla70
Tracking Status
firefox70 --- fixed

People

(Reporter: francois, Assigned: sstreich)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [domsecurity-backlog][wptsync upstream])

Attachments

(1 file)

A number of our CSP violations omit useful details like source files and line numbers:

  this->AsyncReportViolation(aURI,
                             nullptr,       /* originalURI in case of redirect */
                             violatedDirective,
                             i,             /* policy index        */
                             EmptyString(), /* no observer subject */
                             EmptyString(), /* no source file      */
                             EmptyString(), /* no script sample    */
                             0);            /* no line number      */

Source: https://mxr.mozilla.org/mozilla-central/source/dom/security/nsCSPContext.cpp#1110

We should fill those in as much as possible.
Severity: normal → enhancement
Priority: -- → P4
Assignee: nobody → francois
Component: Security → DOM: Security
Local logging -- yay! be careful if we are going to include this information in CSP reports because we've had some Same-Origin violation bugs for giving too much to a potentially hostile reporting site.
Whiteboard: [domsecurity-backlog]
Assignee: francois → nobody
Moving this to the backlog since we're not using P4 anymore.
Priority: P4 → P3

Christoph, it is unclear to me if this affects Console as well or just reporting. If this affects Console, it seems important to fix and it should block bug 1242016, otherwise not. Can you confirm?

Flags: needinfo?(ckerschb)

Is this a case affected by the limitation

  1. Open Console https://firefox-devtools-csp.glitch.me/image
  2. Check the CSP error location

Chrome: image:11, correct line in JS
Firefox: image:1:1, default location for errors?

Summary: Provide more context for CSP violations → Provide source file and location for CSP violations

Continued in bug 1540257 to keep this bug about the general context.

(In reply to :Harald Kirschner :digitarald from comment #3)

Christoph, it is unclear to me if this affects Console as well or just reporting. If this affects Console, it seems important to fix and it should block bug 1242016, otherwise not. Can you confirm?

Currently our 'csp reporting', 'event firing' as well as 'console messages' consume the same information in the backend [1]. So we need to be careful to avoid any SOP violation issues where we accidentally provide too much information. In other words, we probably should separate the issues for local logging and CSP reporting to avoid any issues around that problem.

[1] https://searchfox.org/mozilla-central/source/dom/security/nsCSPContext.cpp#1198

Flags: needinfo?(ckerschb)

Just to re-confirm: This bus is focused on making the Console messages for CSP more useful. If we make them less useful because we are sending the same information to content; this is a problem. Should we treat this in this bug or file a new bug about treating the two warnings targets separately?

Flags: needinfo?(ckerschb)

(In reply to :Harald Kirschner :digitarald from comment #7)

Just to re-confirm: This bus is focused on making the Console messages for CSP more useful. If we make them less useful because we are sending the same information to content; this is a problem. Should we treat this in this bug or file a new bug about treating the two warnings targets separately?

I think for this particular problem we can fix everything within this bug, though we need to be careful. We should just pass more information for inline violations which we currently do not do.

Basti can you take a look?

Assignee: nobody → streich.mobile
Status: NEW → ASSIGNED
Flags: needinfo?(ckerschb) → needinfo?(streich.mobile)
Flags: needinfo?(streich.mobile)
Keywords: checkin-needed

Pushed by dvarga@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/8b0c5e44d937
Print Related JS-Line on CSP Violation (if any) r=ckerschb,dveditz

Keywords: checkin-needed

Backed out changeset 8b0c5e44d937 (bug 1100630) for wpt failures at securitypolicyviolation/securitypolicyviolation-block-cross-origin-image-from-script.sub.html

Backout: https://hg.mozilla.org/integration/autoland/rev/5a6a99018a649c2c8eed05219fc72194a1ac1cf9

Failure push: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=8b0c5e44d937513e87b21fc0bc428b8b00adc0ea

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=256927256&repo=autoland&lineNumber=9958

13:07:31 INFO - TEST-START | /content-security-policy/securitypolicyviolation/securitypolicyviolation-block-cross-origin-image-from-script.sub.html
13:07:31 INFO - Closing window 27917287425
13:07:31 INFO - PID 4124 | [Child 6012, Main Thread] WARNING: No active window: file z:/build/build/sr[Parent 4c140, Main Thread] W/ARNING: Constructjs/xpconneing Rangct/src/XPCJSContext.cpp, line 6e6B2oundary with invalid value: 'mRef || aOffset == 0', file z:/buil
13:07:31 INFO - PID 4124 | d/build/src/obj-firefox/dist/include\mozilla/RangeBoundary.h, line 79
13:07:31 INFO - PID 4124 | [Parent 4140, Main Thread] WARNING: Constructing RangeBoundary with invalid value: 'mRef || aOffset == 0', file z:/build/build/src/obj-firefox/dist/include\mozilla/RangeBoundary.h, line 79
13:07:31 INFO - PID 4124 | --DOCSHELL 085C0C00 == 1 [pid = 4480] [id = {291d2f06-39d7-4d08-814d-d0daa4fa3816}] [url = about:blank]
13:07:31 INFO - PID 4124 | [Parent 4140, Gecko_IOThread] WARNING: pipe error: 109: file z:/build/build/src/ipc/chromium/src/chrome/common/ipc_channel_win.cc, line 341
13:07:31 INFO - PID 4124 | [Child 4480, Chrome_ChildThread] WARNING: pipe error: 109: file z:/build/build/src/ipc/chromium/src/chrome/common/ipc_channel_win.cc, line 341
13:07:31 INFO - PID 4124 | [Child 4480, Chrome_ChildThread] WARNING: pipe error: 109: file z:/build/build/src/ipc/chromium/src/chrome/common/ipc_channel_win.cc, line 341
13:07:31 INFO - PID 4124 | [Parent 4140, Gecko_IOThread] WARNING: pipe error: 109: file z:/build/build/src/ipc/chromium/src/chrome/common/ipc_channel_win.cc, line 341
13:07:31 INFO - PID 4124 | [Parent 4140, Gecko_IOThread] WARNING: pipe error: 109: file z:/build/build/src/ipc/chromium/src/chrome/common/ipc_channel_win.cc, line 341
13:07:31 INFO - PID 4124 | [Parent 4140, Gecko_IOThread] WARNING: pipe error: 109: file z:/build/build/src/ipc/chromium/src/chrome/common/ipc_channel_win.cc, line 341
13:07:31 INFO - PID 4124 | [Parent 4140, Gecko_IOThread] WARNING: pipe error: 109: file z:/build/build/src/ipc/chromium/src/chrome/common/ipc_channel_win.cc, line 341
13:07:31 INFO - PID 4124 | [Parent 4140, Gecko_IOThread] WARNING: pipe error: 109: file z:/build/build/src/ipc/chromium/src/chrome/common/ipc_channel_win.cc, line 341
13:07:31 INFO - PID 4124 | ++DOCSHELL 00B61C00 == 1 [pid = 4528] [id = {c888a18b-194f-4ad9-884b-6cd56a5e894d}]
13:07:31 INFO - PID 4124 | [Child 4528, Main Thread] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/widget/windows/nsLookAndFeel.cpp, line 853
13:07:31 INFO - PID 4124 | ++DOMWINDOW == 1 (00BB9280) [pid = 4528] [serial = 1] [outer = 00000000]
13:07:31 INFO - PID 4124 | [Child 4528, Main Thread] WARNING: No active window: file z:/build/build/src/js/xpconnect/src/XPCJSContext.cpp, line 662
13:07:31 INFO - PID 4124 | [Child 4528, Main Thread] WARNING: No active window: file z:/build/build/src/js/xpconnect/src/XPCJSContext.cpp, line 662
13:07:31 INFO - PID 4124 | [Parent 4140, Main Thread] WARNING: Constructing RangeBoundary with invalid value: 'mRef || aOffset == 0', file z:/build/build/src/obj-firefox/dist/include\mozilla/RangeBoundary.h, line 79
13:07:31 INFO - PID 4124 | [Parent 4140, Main Thread] WARNING: Constructing RangeBoundary with invalid value: 'mRef || aOffset == 0', file z:/build/build/src/obj-firefox/dist/include\mozilla/RangeBoundary.h, line 79
13:07:31 INFO - PID 4124 | ++DOMWINDOW == 2 (00B6A000) [pid = 4528] [serial = 2] [outer = 00BB9280]
13:07:31 INFO - PID 4124 | --DOCSHELL 00B61C00 == 0 [pid = 4480] [id = {a7878d0a-7775-4397-9d10-8c415c85e1f6}] [url = http://web-platform.test:8000/content-security-policy/securitypolicyviolation/script-sample.html]
13:07:31 INFO - PID 4124 | --DOMWINDOW == 3 (00BB7B80) [pid = 4480] [serial = 5] [outer = 00000000] [url = about:blank]
13:07:31 INFO - PID 4124 | --DOMWINDOW == 2 (00BB7280) [pid = 4480] [serial = 1] [outer = 00000000] [url = http://web-platform.test:8000/content-security-policy/securitypolicyviolation/script-sample.html]
13:07:31 INFO - PID 4124 | --DOMWINDOW == 1 (0927E800) [pid = 4480] [serial = 6] [outer = 00000000] [url = about:blank]
13:07:31 INFO - PID 4124 | --DOMWINDOW == 0 (0927B000) [pid = 4480] [serial = 4] [outer = 00000000] [url = http://web-platform.test:8000/content-security-policy/securitypolicyviolation/script-sample.html]
13:07:31 INFO - PID 4124 | [Child 4528, Main Thread] WARNING: 'NS_FAILED(GetAccentColor(unused))', file z:/build/build/src/widget/windows/nsLookAndFeel.cpp, line 481
13:07:31 INFO - PID 4124 | [Child 4480, Main Thread] WARNING: XPCOM objects created/destroyed from static ctor/dtor: file z:/build/build/src/xpcom/base/nsTraceRefcnt.cpp, line 194
13:07:31 INFO - PID 4124 | [Child 4480, Main Thread] WARNING: XPCOM objects created/destroyed from static ctor/dtor: file z:/build/build/src/xpcom/base/nsTraceRefcnt.cpp, line 194
13:07:31 INFO - PID 4124 | nsStringStats
13:07:31 INFO - PID 4124 | => mAllocCount: 8045
13:07:31 INFO - PID 4124 | => mReallocCount: 0
13:07:31 INFO - PID 4124 | => mFreeCount: 8045
13:07:31 INFO - PID 4124 | => mShareCount: 7548
13:07:31 INFO - PID 4124 | => mAdoptCount: 603
13:07:31 INFO - PID 4124 | => mAdoptFreeCount: 609
13:07:31 INFO - PID 4124 | => Process ID: 4480, Thread ID: 4948
13:07:31 INFO - PID 4124 | ++DOMWINDOW == 3 (082C2800) [pid = 4528] [serial = 3] [outer = 00BB9280]
13:07:31 INFO - PID 4124 | ### XPCOM_MEM_BLOAT_LOG defined -- logging bloat/leaks to c:\users\task_1563365082\appdata\local\temp\tmp55zcnv.mozrunner\runtests_leaks_1268_tab_pid4696.log
13:07:31 INFO - PID 4124 | [Child 4528, Main Thread] WARNING: NS_ENSURE_SUCCESS(mStatus, *this) failed with result 0x80004005: file z:/build/build/src/obj-firefox/dist/include\nsIURIMutator.h, line 489
13:07:31 INFO - PID 4124 | ++DOMWINDOW == 4 (09B7C000) [pid = 4528] [serial = 4] [outer = 00BB9280]
13:07:31 INFO - PID 4124 | [Parent 4140, Main Thread] WARNING: Constructing RangeBoundary with invalid value: 'mRef || aOffset == 0', file z:/build/build/src/obj-firefox/dist/include\mozilla/RangeBoundary.h, line 79
13:07:31 INFO - PID 4124 | [Parent 4140, Main Thread] WARNING: Constructing RangeBoundary with invalid value: 'mRef || aOffset == 0', file z:/build/build/src/obj-firefox/dist/include\mozilla/RangeBoundary.h, line 79
13:07:32 INFO - PID 4124 | [Child 4528, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x805E0006: file z:/build/build/src/dom/security/nsContentSecurityManager.cpp, line 949
13:07:32 INFO - PID 4124 | [Child 4528, Main Thread] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/netwerk/protocol/http/HttpChannelChild.cpp, line 2551
13:07:32 INFO - PID 4124 | [Child 4528, Main Thread] WARNING: NS_ENSURE_SUCCESS(rv, rv) failed with result 0x805E0006: file z:/build/build/src/dom/security/nsContentSecurityManager.cpp, line 949
13:07:32 INFO - PID 4124 | [Child 4528, Main Thread] WARNING: 'NS_FAILED(rv)', file z:/build/build/src/netwerk/protocol/http/HttpChannelChild.cpp, line 2551
13:07:32 INFO -
13:07:32 INFO - TEST-UNEXPECTED-FAIL | /content-security-policy/securitypolicyviolation/securitypolicyviolation-block-cross-origin-image-from-script.sub.html | Non-redirected cross-origin URLs are not stripped. - assert_equals: expected "" but got "http://www2.web-platform.test:8000/content-security-policy/support/inject-image.sub.js"
13:07:32 INFO - @http://web-platform.test:8000/content-security-policy/securitypolicyviolation/securitypolicyviolation-block-cross-origin-image-from-script.sub.html:18:9
13:07:32 INFO - Test.prototype.step@http://web-platform.test:8000/resources/testharness.js:1594:25
13:07:32 INFO - Test.prototype.step_func_done/<@http://web-platform.test:8000/resources/testharness.js:1634:32

Flags: needinfo?(streich.mobile)

Hey, i updated the Patch and the tests are passing again.
Should work now :)

Flags: needinfo?(streich.mobile)
Keywords: checkin-needed

Pushed by csabou@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a98b795c2b3c
Print Related JS-Line on CSP Violation (if any) r=ckerschb,dveditz

Keywords: checkin-needed

Basti, third time's a charm

Flags: needinfo?(ckerschb) → needinfo?(streich.mobile)
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/17916 for changes under testing/web-platform/tests
Whiteboard: [domsecurity-backlog] → [domsecurity-backlog][wptsync upstream]
Upstream PR was closed without merging
Flags: needinfo?(streich.mobile)
Keywords: checkin-needed

Pushed by ccoroiu@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c80e50cc1a73
Print Related JS-Line on CSP Violation (if any) r=ckerschb,dveditz

Keywords: checkin-needed
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla70
Upstream PR merged
You need to log in before you can comment on or make changes to this bug.