Closed Bug 1507991 Opened 6 years ago Closed 6 years ago

Crash in PermissionMessageUtils.cpp when loading a moz-icon: uri

Categories

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

Unspecified
All
defect

Tracking

()

VERIFIED FIXED
mozilla67
Tracking Status
firefox-esr60 --- unaffected
firefox63 --- unaffected
firefox64 --- disabled
firefox65 --- wontfix
firefox66 --- wontfix
firefox67 --- verified

People

(Reporter: pauljt, Assigned: nika)

References

Details

(Keywords: crash, reproducible)

Crash Data

Attachments

(3 files, 2 obsolete files)

This is an odd edge case, but if you paste a moz-icon: url with a file:uri into the address bar, it causes a crash in PermissionMessageUtils.cpp[2]. It's just a mozcrash, and it doesn't look dangerous to me (web pages can't load or link to these urls so it would be hard to induce in any case). STR: 1. Load moz-icon:file:///... uri such as [1] in a tab 2. icon from exe with load, then wait a second or so 3. tab will crash after a second or so Reproduces in release (build id: 20181112164519) and nightly 64.0b9 (20181112164519) [1] moz-icon:file:///C:/Users/User/AppData/Local/Programs/Microsoft VS Code/Code.exe?size=16 [2] https://hg.mozilla.org/releases/mozilla-beta/annotate/6226ce7f4038d566fe750b1d13f107146d137463/dom/ipc/PermissionMessageUtils.cpp#l25 This bug was filed from the Socorro interface and is report bp-b10e024d-38cc-40f9-a672-3119b0181116. ============================================================= Top 10 frames of crashing thread: 0 xul.dll IPC::ParamTraits<nsIPrincipal>::Write dom/ipc/PermissionMessageUtils.cpp:25 1 xul.dll mozilla::dom::PContentChild::SendStoreUserInteractionAsPermission ipc/ipdl/PContentChild.cpp:5068 2 xul.dll mozilla::AntiTrackingCommon::StoreUserInteractionFor toolkit/components/antitracking/AntiTrackingCommon.cpp:1312 3 xul.dll nsIDocument::SetUserHasInteracted dom/base/nsDocument.cpp:12896 4 xul.dll mozilla::EventStateManager::PreHandleEvent dom/events/EventStateManager.cpp:512 5 xul.dll nsresult mozilla::PresShell::HandleEventInternal layout/base/PresShell.cpp:7661 6 xul.dll mozilla::PresShell::HandleEventWithTarget layout/base/PresShell.cpp:7480 7 xul.dll mozilla::PointerEventHandler::DispatchPointerFromMouseOrTouch dom/events/PointerEventHandler.cpp:535 8 xul.dll mozilla::PresShell::HandleEvent layout/base/PresShell.cpp:7257 9 xul.dll nsViewManager::DispatchEvent view/nsViewManager.cpp:812 =============================================================
Component: IPC → Security: CAPS
:Nika Layzell Your bunch of patch seems to cause the crash Nightly65.0a1 and Thunderbird Daily65.0a1 as well. Can you please look into this?
Flags: needinfo?(nika)
(BTW, It seems to work without crash on Firefox 64. )
report bp-456d3fe7-5ffe-4cee-914a-be8680181206. ============================================================= Top 10 frames of crashing thread: 0 xul.dll IPC::ParamTraits<nsIPrincipal>::Write dom/ipc/PermissionMessageUtils.cpp:23 1 xul.dll mozilla::ipc::IPDLParamTraits<mozilla::dom::WindowGlobalInit>::Write ipc/ipdl/DOMTypes.cpp:1905 2 xul.dll mozilla::dom::PBrowserChild::SendPWindowGlobalConstructor ipc/ipdl/PBrowserChild.cpp:337 3 xul.dll mozilla::dom::WindowGlobalChild::Create dom/ipc/WindowGlobalChild.cpp:60 4 xul.dll nsGlobalWindowInner::InnerSetNewDocument dom/base/nsGlobalWindowInner.cpp:1651 5 xul.dll nsGlobalWindowOuter::SetNewDocument dom/base/nsGlobalWindowOuter.cpp:1939 6 xul.dll nsresult nsDocumentViewer::InitInternal layout/base/nsDocumentViewer.cpp:969 7 xul.dll nsDocumentViewer::Init layout/base/nsDocumentViewer.cpp:715 8 xul.dll nsresult nsDocShell::Embed docshell/base/nsDocShell.cpp:6357 9 xul.dll nsDSURIContentListener::DoContent docshell/base/nsDSURIContentListener.cpp:182 =============================================================
I hit this 3 times in a row attempting to view an email in Thunderbird Daily 20181206075341 signed Nightly build on Linux x86_64. MOZ_CRASH(Unable to serialize principal.) bp-615edf4b-8ad2-4234-94f1-1c3b90181206 bp-9ced073a-c52f-4330-b36a-577e90181206 bp-5e484e13-b304-4e82-9175-c3ded0181206
OS: Windows 10 → All
Hmm, so it appears as though there are some principal serialization issues. I'll look into putting a quick fix together. The Thunderbird issue is distinct, they also have some principals which cannot be serialized currently unfortunately. I don't consider their bugs to be part of that issue.
This is needed to use the IPDLParamTraits implementation for nsIURI which is used in part 2 of this patch series.
This should add support for serializing moz-icon:// URIs and other URIs which don't implement nsISerializable, but do implement nsIIPCSerializableURI. This also avoids the continued use of nsISerializable, which is intended for persistance, for IPC, saving unnecessary byte-order swaps etc. Depends on D14434
Assignee: nobody → nika
Flags: needinfo?(nika)
Priority: -- → P2

This is needed to use the IPDLParamTraits implementation for nsIURI which is
used in part 2 of this patch series.

This is needed to maintain full feature parity with the existing
nsIPrincipal serializer while switching to using the PrincipalInfo-based
one.

Depends on D20853

Attachment #9046002 - Attachment is obsolete: true
Attachment #9031148 - Attachment is obsolete: true
Pushed by nlayzell@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/a483c170d712 Part 1: Use IPDLParamTraits for nsIPrincipal, r=mccr8 https://hg.mozilla.org/integration/autoland/rev/15116b423375 Part 2: Serialize domain in ContentPrincipalInfo, r=baku https://hg.mozilla.org/integration/autoland/rev/71c093a6dace Part 3: Serialize nsIPrincipal using PrincipalInfo, r=baku

Backed out 3 changesets (bug 1507991) for bustages at DBSchema.cpp on a CLOSED TREE.

Backout link: https://hg.mozilla.org/integration/autoland/rev/177cf2f812d59db13ad3f7fde4dfa227fe03de21

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&revision=71c093a6dace9b1b4f376ae623b38f6e7f2fcbbe&selectedJob=230617048

Log link: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=230617048&repo=autoland&lineNumber=18063

Log snippet:

[task 2019-02-26T20:44:14.203Z] 20:44:14 INFO - make[5]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/gfx/skia'
[task 2019-02-26T20:44:14.203Z] 20:44:14 INFO - gfx/skia/GrOvalEffect.i_o
[task 2019-02-26T20:44:14.203Z] 20:44:14 INFO - make[5]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/gfx/skia'
[task 2019-02-26T20:44:15.027Z] 20:44:15 INFO - make[5]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/dom/cache'
[task 2019-02-26T20:44:15.028Z] 20:44:15 INFO - /builds/worker/workspace/build/src/clang/bin/clang++ -m32 -o Unified_cpp_dom_cache1.i_o -c -I/builds/worker/workspace/build/src/obj-firefox/dist/stl_wrappers -I/builds/worker/workspace/build/src/obj-firefox/dist/system_wrappers -include /builds/worker/workspace/build/src/config/gcc_hidden.h -DNDEBUG=1 -DTRIMMED=1 -DOS_POSIX=1 -DOS_LINUX=1 -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/builds/worker/workspace/build/src/dom/cache -I/builds/worker/workspace/build/src/obj-firefox/dom/cache -I/builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/_ipdlheaders -I/builds/worker/workspace/build/src/ipc/chromium/src -I/builds/worker/workspace/build/src/ipc/glue -I/builds/worker/workspace/build/src/obj-firefox/dist/include -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nspr -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/build/src/obj-firefox/mozilla-config.h -Qunused-arguments -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -Qunused-arguments -Wall -Wbitfield-enum-conversion -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wc++1z-compat -Wc++2a-compat -Wcomma -Wimplicit-fallthrough -Werror=non-literal-null-conversion -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=return-std-move -Wno-error=atomic-alignment -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Wno-unknown-warning-option -Wno-return-type-c-linkage -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -fcrash-diagnostics-dir=/builds/worker/artifacts -march=pentium-m -msse -msse2 -mfpmath=sse -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -Xclang -load -Xclang /builds/worker/workspace/build/src/obj-firefox/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -O3 -fno-omit-frame-pointer -funwind-tables -Werror -Wno-error=shadow -fprofile-instr-generate -MD -MP -MF .deps/Unified_cpp_dom_cache1.i_o.pp /builds/worker/workspace/build/src/obj-firefox/dom/cache/Unified_cpp_dom_cache1.cpp
[task 2019-02-26T20:44:15.028Z] 20:44:15 INFO - In file included from /builds/worker/workspace/build/src/obj-firefox/dom/cache/Unified_cpp_dom_cache1.cpp:11:
[task 2019-02-26T20:44:15.028Z] 20:44:15 ERROR - /builds/worker/workspace/build/src/dom/cache/DBSchema.cpp:2498:9: error: no matching constructor for initialization of 'mozilla::ipc::ContentPrincipalInfo'
[task 2019-02-26T20:44:15.028Z] 20:44:15 INFO - mozilla::ipc::ContentPrincipalInfo(attrs, origin, specNoSuffix,
[task 2019-02-26T20:44:15.029Z] 20:44:15 INFO - ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
[task 2019-02-26T20:44:15.029Z] 20:44:15 INFO - /builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/_ipdlheaders/mozilla/ipc/PBackgroundSharedTypes.h:142:18: note: candidate constructor not viable: requires 5 arguments, but 4 were provided
[task 2019-02-26T20:44:15.029Z] 20:44:15 INFO - MOZ_IMPLICIT ContentPrincipalInfo(
[task 2019-02-26T20:44:15.029Z] 20:44:15 INFO - ^
[task 2019-02-26T20:44:15.029Z] 20:44:15 INFO - /builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/_ipdlheaders/mozilla/ipc/PBackgroundSharedTypes.h:126:7: note: candidate constructor (the implicit copy constructor) not viable: requires 1 argument, but 4 were provided
[task 2019-02-26T20:44:15.029Z] 20:44:15 INFO - class ContentPrincipalInfo final
[task 2019-02-26T20:44:15.030Z] 20:44:15 INFO - ^
[task 2019-02-26T20:44:15.031Z] 20:44:15 INFO - /builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/_ipdlheaders/mozilla/ipc/PBackgroundSharedTypes.h:126:7: note: candidate constructor (the implicit move constructor) not viable: requires 1 argument, but 4 were provided
[task 2019-02-26T20:44:15.031Z] 20:44:15 INFO - /builds/worker/workspace/build/src/obj-firefox/ipc/ipdl/_ipdlheaders/mozilla/ipc/PBackgroundSharedTypes.h:133:18: note: candidate constructor not viable: requires 0 arguments, but 4 were provided
[task 2019-02-26T20:44:15.032Z] 20:44:15 INFO - MOZ_IMPLICIT ContentPrincipalInfo() :
[task 2019-02-26T20:44:15.032Z] 20:44:15 INFO - ^
[task 2019-02-26T20:44:15.032Z] 20:44:15 INFO - 1 error generated.
[task 2019-02-26T20:44:15.033Z] 20:44:15 INFO - /builds/worker/workspace/build/src/config/rules.mk:805: recipe for target 'Unified_cpp_dom_cache1.i_o' failed
[task 2019-02-26T20:44:15.034Z] 20:44:15 ERROR - make[5]: *** [Unified_cpp_dom_cache1.i_o] Error 1
[task 2019-02-26T20:44:15.034Z] 20:44:15 INFO - make[5]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/dom/cache'
[task 2019-02-26T20:44:15.035Z] 20:44:15 INFO - /builds/worker/workspace/build/src/config/recurse.mk:74: recipe for target 'dom/cache/target' failed
[task 2019-02-26T20:44:15.035Z] 20:44:15 ERROR - make[4]: *** [dom/cache/target] Error 2
[task 2019-02-26T20:44:15.036Z] 20:44:15 INFO - make[4]: *** Waiting for unfinished jobs....
[task 2019-02-26T20:44:15.036Z] 20:44:15 INFO - make[5]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/gfx/skia'
[task 2019-02-26T20:44:15.036Z] 20:44:15 INFO - gfx/skia/GrPorterDuffXferProcessor.i_o
[task 2019-02-26T20:44:15.037Z] 20:44:15 INFO - make[5]: Leaving directory '/builds/worker/workspace/build/src/obj-firefox/gfx/skia'
[task 2019-02-26T20:44:17.373Z] 20:44:17 INFO - make[5]: Entering directory '/builds/worker/workspace/build/src/obj-firefox/gfx/skia'
[task 2019-02-26T20:44:17.373Z] 20:44:17 INFO - /builds/worker/workspace/build/src/clang/bin/clang++ -m32 -o GrOvalEffect.i_o -c -I/builds/worker/workspace/build/src/obj-firefox/dist/stl_wrappers -I/builds/worker/workspace/build/src/obj-firefox/dist/system_wrappers -include /builds/worker/workspace/build/src/config/gcc_hidden.h -DNDEBUG=1 -DTRIMMED=1 -DSKIA_IMPLEMENTATION=1 -DSK_PDF_USE_SFNTLY=1 -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -I/builds/worker/workspace/build/src/gfx/skia -I/builds/worker/workspace/build/src/obj-firefox/gfx/skia -I/builds/worker/workspace/build/src/gfx/skia/skia/include/c -I/builds/worker/workspace/build/src/gfx/skia/skia/include/codec -I/builds/worker/workspace/build/src/gfx/skia/skia/include/config -I/builds/worker/workspace/build/src/gfx/skia/skia/include/core -I/builds/worker/workspace/build/src/gfx/skia/skia/include/docs -I/builds/worker/workspace/build/src/gfx/skia/skia/include/effects -I/builds/worker/workspace/build/src/gfx/skia/skia/include/encode -I/builds/worker/workspace/build/src/gfx/skia/skia/include/gpu -I/builds/worker/workspace/build/src/gfx/skia/skia/include/pathops -I/builds/worker/workspace/build/src/gfx/skia/skia/include/ports -I/builds/worker/workspace/build/src/gfx/skia/skia/include/private -I/builds/worker/workspace/build/src/gfx/skia/skia/include/utils -I/builds/worker/workspace/build/src/gfx/skia/skia/include/utils/mac -I/builds/worker/workspace/build/src/gfx/skia/skia/src/codec -I/builds/worker/workspace/build/src/gfx/skia/skia/src/core -I/builds/worker/workspace/build/src/gfx/skia/skia/src/gpu -I/builds/worker/workspace/build/src/gfx/skia/skia/src/gpu/effects -I/builds/worker/workspace/build/src/gfx/skia/skia/src/gpu/gl -I/builds/worker/workspace/build/src/gfx/skia/skia/src/gpu/glsl -I/builds/worker/workspace/build/src/gfx/skia/skia/src/image -I/builds/worker/workspace/build/src/gfx/skia/skia/src/lazy -I/builds/worker/workspace/build/src/gfx/skia/skia/src/opts -I/builds/worker/workspace/build/src/gfx/skia/skia/src/sfnt -I/builds/worker/workspace/build/src/gfx/skia/skia/src/shaders -I/builds/worker/workspace/build/src/gfx/skia/skia/src/shaders/gradients -I/builds/worker/workspace/build/src/gfx/skia/skia/src/sksl -I/builds/worker/workspace/build/src/gfx/skia/skia/src/utils -I/builds/worker/workspace/build/src/gfx/skia/skia/src/utils/mac -I/builds/worker/workspace/build/src/gfx/skia/skia/src/utils/win -I/builds/worker/workspace/build/src/gfx/sfntly/cpp/src -I/builds/worker/workspace/build/src/obj-firefox/dist/include -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nspr -I/builds/worker/workspace/build/src/obj-firefox/dist/include/nss -fPIC -DMOZILLA_CLIENT -include /builds/worker/workspace/build/src/obj-firefox/mozilla-config.h -Qunused-arguments -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -Qunused-arguments -Wall -Wbitfield-enum-conversion -Wempty-body -Wignored-qualifiers -Woverloaded-virtual -Wpointer-arith -Wshadow-field-in-constructor-modified -Wsign-compare -Wtype-limits -Wunreachable-code -Wunreachable-code-return -Wwrite-strings -Wno-invalid-offsetof -Wclass-varargs -Wfloat-overflow-conversion -Wfloat-zero-conversion -Wloop-analysis -Wc++1z-compat -Wc++2a-compat -Wcomma -Wimplicit-fallthrough -Werror=non-literal-null-conversion -Wstring-conversion -Wtautological-overlap-compare -Wtautological-unsigned-enum-zero-compare -Wtautological-unsigned-zero-compare -Wno-inline-new-delete -Wno-error=deprecated-declarations -Wno-error=array-bounds -Wno-error=return-std-move -Wno-error=atomic-alignment -Wformat -Wformat-security -Wno-gnu-zero-variadic-macro-arguments -Wno-unknown-warning-option -Wno-return-type-c-linkage -D_GLIBCXX_USE_CXX11_ABI=0 -fno-sized-deallocation -fcrash-diagnostics-dir=/builds/worker/artifacts -march=pentium-m -msse -msse2 -mfpmath=sse -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -fstack-protector-strong -fno-exceptions -fno-strict-aliasing -fno-rtti -ffunction-sections -fdata-sections -fno-exceptions -fno-math-errno -pthread -pipe -g -Xclang -load -Xclang /builds/worker/workspace/build/src/obj-firefox/build/clang-plugin/libclang-plugin.so -Xclang -add-plugin -Xclang moz-check -O3 -fno-omit-frame-pointer -funwind-tables -Wno-deprecated-declarations -Wno-overloaded-virtual -Wno-shadow -Wno-sign-compare -Wno-unreachable-code -Wno-unused-function -Wno-implicit-fallthrough -Wno-inconsistent-missing-override -Wno-macro-redefined -Wno-unused-private-field -I/builds/worker/workspace/build/src/obj-firefox/dist/include/cairo -I/usr/include/freetype2 -pthread -I/usr/include/pango-1.0 -I/usr/include/glib-2.0 -I/usr/lib/i386-linux-gnu/glib-2.0/include -I/usr/include/harfbuzz -I/usr/include/freetype2 -I/usr/include/cairo -I/usr/include/pixman-1 -I/usr/include/libpng12 -fprofile-instr-generate -MD -MP -MF .deps/GrOvalEffect.i_o.pp /builds/worker/workspace/build/src/gfx/skia/skia/src/gpu/effects/GrOvalEffect.cpp
[task 2019-02-26T20:44:17.374Z] 20:44:17 WARNING - /builds/worker/workspace/build/src/gfx/skia/skia/src/gpu/effects/GrOvalEffect.cpp:32:12: warning: 'return' will never be executed [-Wunreachable-code-return]
[task 2019-02-26T20:44:17.374Z] 20:44:17 INFO - return nullptr;

Flags: needinfo?(nika)

Oops - seems like I landed an older version of the patch stack :-S - gonna try to re-land with the more up-to-date version.

Flags: needinfo?(nika)
Pushed by nlayzell@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/58296c56129b Part 1: Use IPDLParamTraits for nsIPrincipal, r=mccr8 https://hg.mozilla.org/integration/autoland/rev/f02a51eb1d7b Part 2: Serialize domain in ContentPrincipalInfo, r=baku https://hg.mozilla.org/integration/autoland/rev/e865352417c9 Part 3: Serialize nsIPrincipal using PrincipalInfo, r=baku
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla67

Low crash volume, I guess this can ride the trains.

I'm confused about why the domain doesn't get serialized/deserialized by WritePrincipalInfo/ReadPrincipalInfo. Why add it to PrincipalInfo if it only survives a PrincipalInfoToPrincipal(PrincipalToPrincipalInfo(foo)) round-trip that does not cross processes via IPC? Is it a policy decision because document.domain is dumb?

Flags: needinfo?(nika)

(In reply to Andrew Sutherland [:asuth] from comment #19)

I'm confused about why the domain doesn't get serialized/deserialized by WritePrincipalInfo/ReadPrincipalInfo. Why add it to PrincipalInfo if it only survives a PrincipalInfoToPrincipal(PrincipalToPrincipalInfo(foo)) round-trip that does not cross processes via IPC? Is it a policy decision because document.domain is dumb?

It does survive when crossing IPC, but it doesn't survive being written to Structured Clone from JS. That's more of a "I meant to get to it but forgot" issue, rather than an intentional issue.

I really want to move document.domain out of the principal object entirely at some point (IMO principal should be immutable & threadsafe), but while it's still in there we should probably preserve it there.

Flags: needinfo?(nika)
Flags: qe-verify+

Reproduced the initial crash using 64.0b9 on Windows 10 https://crash-stats.mozilla.com/report/index/b529dbf0-cd55-46fe-adef-237790190423
Verified that latest Beta 67.0b13 and latest Nightly 68.0a1 does not receive any crashes following the steps from this bug across platforms (Windows 10 64bit, Ubuntu 18.04 64bit and macOS 10.14).

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: