Closed Bug 1499355 Opened Last year Closed Last year

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

Categories

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

defect

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox64 --- fixed

People

(Reporter: jdescottes, Assigned: ckerschb)

References

Details

(Whiteboard: [domsecurity-active])

Attachments

(3 files)

Attached patch test-case.patchSplinter 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: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.