Closed Bug 1499355 Opened 6 years ago Closed 6 years ago

Null pointer exception when adding CSP to about: page index.html

Categories

(Core :: DOM: Security, defect, P2)

Product:
Core
Component:
DOM: Security
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla64
Tracking Flags:
Tracking Status
firefox64 --- fixed

People

(Reporter: jdescottes, Assigned: ckerschb)

References

Details

(Whiteboard: [domsecurity-active])

Attachments

(3 files)

test-case.patch
2.61 KB, patch
Details | Diff | Splinter Review
crash_logs.txt
10.59 KB, text/plain
Details
bug_1499355_ensurepreloadcsp_error.patch
900 bytes, patch
baku
: review+
Details | Diff | Splinter Review
Attached patch test-case.patch — — Splinter Review
STRs: - apply the patch in attachment - build and run Firefox - in about:config, check that devtools.aboutdebugging.new-enabled is set to true - open about:debugging Firefox crashes immediately for me. :ckerschb could not reproduce the issue so this might be OSX only. Will attach logs.
Attached file crash_logs.txt —
If you edit devtools/client/aboutdebugging-new/index.html to remove the CSP meta and run the scenario again, you should be able to have a blank page for "about:debugging". For reference the current markup of this index.html file is: <!-- This Source Code Form is subject to the terms of the Mozilla Public - License, v. 2.0. If a copy of the MPL was not distributed with this - file, You can obtain one at http://mozilla.org/MPL/2.0/. --> <!DOCTYPE html> <html> <head> <meta http-equiv="Content-Security-Policy" content="default-src chrome: resource:" /> </head> <body> <div id="mount"></div> </body> </html> The registration for this component is made at https://searchfox.org/mozilla-central/source/devtools/startup/aboutdebugging-registration.js
Final thing to mention, this doesn't happen with the equivalent using xhtml.
I figured what the problem is, why that's not happening if using .xhtml I don't know though. Applying a meta-csp to a system privileged about page like about:debugging tries to add a CSP to the SystemPrincipal. In particular, we call AddSpeculationCSP() which then calls EnsurePreloadCSP() on a system principal. SystemPrincipal::EnsurePreloadCSP() simply returns NS_OK and we try to deref the nullptr CSP and hence crash. Two options how to fix: (a) We add an explicit null check before dereferencing the CSP within AddSpeculationCSP(), (b) We wait till Bug 965637 is fixed, we removes all of that EnsurePreloadCSP code. I am for (b) and we should just wait till Bug 965637 is fixed. Please note that no website can trigger this code on a SystemPrincipal, that can only happen on one of our own sites. [1] https://searchfox.org/mozilla-central/source/parser/html/nsHtml5TreeOpExecutor.cpp#1146 [2] https://searchfox.org/mozilla-central/source/caps/SystemPrincipal.cpp#105 %----------snip--------------- Thread 1 "firefox" received signal SIGSEGV, Segmentation fault. 0x00007fffe2376772 in nsCOMPtr<nsIContentSecurityPolicy>::operator-> (this=0x7fffffffb650) at /home/ckerschb/source/mc-dbg/dist/include/nsCOMPtr.h:939 939 MOZ_ASSERT(mRawPtr != nullptr, (gdb) --DOCSHELL 0x7fffd6d10000 == 0 [pid = 2017] [id = {e88f292e-5ba5-4835-b525-eb72fe686a28}] --DOMWINDOW == 1 (0x7fffd6bfb200) [pid = 2017] [serial = 1] [outer = (nil)] [url = about:home] bt--DOMWINDOW == 0 (0x7fffd6727800) [pid = 2017] [serial = 3] [outer = (nil)] [url = about:home] #0 0x00007fffe2376772 in nsCOMPtr<nsIContentSecurityPolicy>::operator->() const (this=0x7fffffffb650) at /home/ckerschb/source/mc-dbg/dist/include/nsCOMPtr.h:939 #1 0x00007fffe38684f6 in nsHtml5TreeOpExecutor::AddSpeculationCSP(nsTSubstring<char16_t> const&) (this=0x7fffcbdb6800, aCSP=...) at /home/ckerschb/source/mc/parser/html/nsHtml5TreeOpExecutor.cpp:1152 #2 0x00007fffe3867d28 in nsHtml5SpeculativeLoad::Perform(nsHtml5TreeOpExecutor*) (this=0x7fffcc9aa910, aExecutor=0x7fffcbdb6800) at /home/ckerschb/source/mc/parser/html/nsHtml5SpeculativeLoad.cpp:39 #3 0x00007fffe3892c79 in nsHtml5TreeOpExecutor::FlushSpeculativeLoads() (this=0x7fffcbdb6800) at /home/ckerschb/source/mc/parser/html/nsHtml5TreeOpExecutor.cpp:338 #4 0x00007fffe38b22f1 in nsHtml5LoadFlusher::Run() (this=0x7fffcbd1d250) at /home/ckerschb/source/mc/parser/html/nsHtml5StreamParser.cpp:142 #5 0x00007fffe2248592 in nsThread::ProcessNextEvent(bool, bool*) (this=0x7fffda69f280, aMayWait=false, aResult=0x7fffffffbf6e) at /home/ckerschb/source/mc/xpcom/threads/nsThread.cpp:1252 #6 0x00007fffe224b66c in NS_ProcessNextEvent(nsIThread*, bool) (aThread=0x7fffda69f280, aMayWait=false) at /home/ckerschb/source/mc/xpcom/threads/nsThreadUtils.cpp:530 #7 0x00007fffe2cbb86f in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) (this=0x7fffda6c59c0, aDelegate=0x7ffff6a5e200) at /home/ckerschb/source/mc/ipc/glue/MessagePump.cpp:97 #8 0x00007fffe2bf73a5 in MessageLoop::RunInternal() (this=0x7ffff6a5e200) at /home/ckerschb/source/mc/ipc/chromium/src/base/message_loop.cc:325 #9 0x00007fffe2bf7325 in MessageLoop::RunHandler() (this=0x7ffff6a5e200) at /home/ckerschb/source/mc/ipc/chromium/src/base/message_loop.cc:318 #10 0x00007fffe2bf72fd in MessageLoop::Run() (this=0x7ffff6a5e200) at /home/ckerschb/source/mc/ipc/chromium/src/base/message_loop.cc:298 #11 0x00007fffe6d1dd23 in nsBaseAppShell::Run() (this=0x7fffd9707900) at /home/ckerschb/source/mc/widget/nsBaseAppShell.cpp:158 #12 0x00007fffe92f1342 in nsAppStartup::Run() (this=0x7fffd9782e20) at /home/ckerschb/source/mc/toolkit/components/startup/nsAppStartup.cpp:290 #13 0x00007fffe9481be2 in XREMain::XRE_mainRun() (this=0x7fffffffca10) at /home/ckerschb/source/mc/toolkit/xre/nsAppRunner.cpp:4777 #14 0x00007fffe9482ca2 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) (this=0x7fffffffca10, argc=4, argv=0x7fffffffddc8, aConfig=...) at /home/ckerschb/source/mc/toolkit/xre/nsAppRunner.cpp:4922 #15 0x00007fffe948351c in XRE_main(int, char**, mozilla::BootstrapConfig const&) (argc=4, argv=0x7fffffffddc8, aConfig=...) at /home/ckerschb/source/mc/toolkit/xre/nsAppRunner.cpp:5014 #16 0x00007fffe949ab87 in mozilla::BootstrapImpl::XRE_main(int, char**, mozilla::BootstrapConfig const&) (this=0x7ffff6a4b6a0, argc=4, argv=0x7fffffffddc8, aConfig=...) at /home/ckerschb/source/mc/toolkit/xre/Bootstrap.cpp:49 #17 0x000055555556d73f in do_main(int, char**, char**) (argc=4, argv=0x7fffffffddc8, envp=0x7fffffffddf0) at /home/ckerschb/source/mc/browser/app/nsBrowserApp.cpp:233 #18 0x000055555556d30e in main(int, char**, char**) (argc=4, argv=0x7fffffffddc8, envp=0x7fffffffddf0) at /home/ckerschb/source/mc/browser/app/nsBrowserApp.cpp:315
Depends on: 965637
Assignee: nobody → ckerschb
Status: NEW → ASSIGNED
Priority: -- → P2
Whiteboard: [domsecurity-active]
Now that I think about it more, we should fix this until we have moved the CSP into the client. @baku: EnsurePreloadCSP() on SystemPrincipal should return NS_ERROR_FAILURE. Please note that EnsureCSP() on SystemPrincipal already returns an error in that case.
Attachment #9017587 - Flags: review?(amarchesini)
Attachment #9017587 - Flags: review?(amarchesini) → review+
Pushed by mozilla@christophkerschbaumer.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/a48759a33738 EnsurePreloadCSP on SystemPrincipal should return error. r=baku
https://hg.mozilla.org/mozilla-central/rev/a48759a33738
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
status-firefox64: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: