Closed
Bug 1408497
Opened 7 years ago
Closed 7 years ago
Stop allowing inotify in Linux content processes
Categories
(Core :: Security: Process Sandboxing, enhancement, P1)
Tracking
()
RESOLVED
FIXED
mozilla58
Tracking | Status | |
---|---|---|
firefox58 | --- | fixed |
People
(Reporter: jld, Assigned: jld)
References
Details
(Whiteboard: sb+)
Attachments
(1 file)
inotify_add_watch takes a filesystem path, which means it bypasses our current sandbox's filesystem restrictions (bug 1308400) and whatever it's being used for will stop working when filesystem access is removed by chroot()ing (bug 1213998).
There are notes that suggest this is used by glib to monitor /usr/share/applications; we should find out where in Gecko this comes from and either remove it if possible or, if it can fail harmlessly, return ENOSYS from inotify_init{1,} and block the rest of the inotify syscalls.
Assignee | ||
Comment 1•7 years ago
|
||
It's the MIME service. One STR (after blocking all the inotify calls in SandboxFilter.cpp), reduced from a reftest, is to load image/test/reftest/bmp/bmp-1bpp/bmp-size-1x1-1bpp.bmp from the source tree via file URL. Stack trace:
Sandbox: seccomp sandbox violation: pid 58878, tid 58878, syscall 294, args 524288 0 0 107 0 140606916743176. Killing process.
Sandbox: crash reporter is disabled (or failed); trying stack trace:
Sandbox: frame #01: __GI_inotify_init1 (/build/glibc-bfm8X4/glibc-2.23/misc/../sysdeps/unix/syscall-template.S:84)
Sandbox: frame #02: ik_source_new (/build/glib2.0-prJhLS/glib2.0-2.48.2/./gio/inotify/inotify-kernel.c:383)
Sandbox: frame #03: _ip_startup (/build/glib2.0-prJhLS/glib2.0-2.48.2/./gio/inotify/inotify-path.c:120)
Sandbox: frame #04: _ih_startup (/build/glib2.0-prJhLS/glib2.0-2.48.2/./gio/inotify/inotify-helper.c:84)
Sandbox: frame #05: try_class (/build/glib2.0-prJhLS/glib2.0-2.48.2/./gio/giomodule.c:653 (discriminator 1))
Sandbox: frame #06: _g_io_module_get_default_type (/build/glib2.0-prJhLS/glib2.0-2.48.2/./gio/giomodule.c:748)
Sandbox: frame #07: g_local_file_monitor_new (/build/glib2.0-prJhLS/glib2.0-2.48.2/./gio/glocalfilemonitor.c:837)
Sandbox: frame #08: g_local_file_monitor_new_in_worker (/build/glib2.0-prJhLS/glib2.0-2.48.2/./gio/glocalfilemonitor.c:881)
Sandbox: frame #09: desktop_file_dir_unindexed_init (/build/glib2.0-prJhLS/glib2.0-2.48.2/./gio/gdesktopappinfo.c:913)
Sandbox: frame #10: g_app_info_get_default_for_type (/build/glib2.0-prJhLS/glib2.0-2.48.2/./gio/gdesktopappinfo.c:4026)
Sandbox: frame #11: nsGIOService::GetAppForMimeType(nsTSubstring<char> const&, nsIGIOMimeApp**) (/home/jld/src/gecko-dev/toolkit/system/gnome/nsGIOService.cpp:296)
Sandbox: frame #12: nsGNOMERegistry::GetFromType(nsTSubstring<char> const&) (/home/jld/src/gecko-dev/uriloader/exthandler/unix/nsGNOMERegistry.cpp:97)
Sandbox: frame #13: nsGNOMERegistry::GetFromExtension(nsTSubstring<char> const&) (/home/jld/src/gecko-dev/uriloader/exthandler/unix/nsGNOMERegistry.cpp:74)
Sandbox: frame #14: nsOSHelperAppService::GetFromExtension(nsTString<char> const&) (/home/jld/src/gecko-dev/uriloader/exthandler/unix/nsOSHelperAppService.cpp:1251)
Sandbox: frame #15: already_AddRefed<nsMIMEInfoBase>::take() (/home/jld/src/obj.gecko-dev/obj-x86_64-pc-linux-gnu/dist/include/mozilla/AlreadyAddRefed.h:121)
Sandbox: frame #16: already_AddRefed<nsIMIMEInfo>::take() (/home/jld/src/obj.gecko-dev/obj-x86_64-pc-linux-gnu/dist/include/mozilla/AlreadyAddRefed.h:121)
Sandbox: frame #17: nsExternalHelperAppService::GetTypeFromExtension(nsTSubstring<char> const&, nsTSubstring<char>&) (/home/jld/src/gecko-dev/uriloader/exthandler/nsExternalHelperAppService.cpp:2715)
Sandbox: frame #18: nsExternalHelperAppService::GetTypeFromFile(nsIFile*, nsTSubstring<char>&) (/home/jld/src/gecko-dev/uriloader/exthandler/nsExternalHelperAppService.cpp:2850)
Sandbox: frame #19: nsFileChannel::MakeFileInputStream(nsIFile*, nsCOMPtr<nsIInputStream>&, nsTString<char>&, bool) (/home/jld/src/gecko-dev/netwerk/protocol/file/nsFileChannel.cpp:357)
Sandbox: frame #20: nsFileChannel::OpenContentStream(bool, nsIInputStream**, nsIChannel**) (/home/jld/src/gecko-dev/netwerk/protocol/file/nsFileChannel.cpp:431)
Sandbox: frame #21: nsCOMPtr<nsIInputStream>::get() const (/home/jld/src/obj.gecko-dev/obj-x86_64-pc-linux-gnu/dist/include/nsCOMPtr.h:780)
Sandbox: frame #22: nsBaseChannel::AsyncOpen(nsIStreamListener*, nsISupports*) (/home/jld/src/gecko-dev/netwerk/base/nsBaseChannel.cpp:720)
Sandbox: frame #23: nsBaseChannel::AsyncOpen2(nsIStreamListener*) (/home/jld/src/gecko-dev/netwerk/base/nsBaseChannel.cpp:746)
Sandbox: frame #24: nsURILoader::OpenURI(nsIChannel*, unsigned int, nsIInterfaceRequestor*) (/home/jld/src/gecko-dev/uriloader/base/nsURILoader.cpp:843)
Sandbox: frame #25: nsDocShell::DoChannelLoad(nsIChannel*, nsIURILoader*, bool) (/home/jld/src/gecko-dev/docshell/base/nsDocShell.cpp:11728)
Sandbox: frame #26: nsDocShell::DoURILoad(nsIURI*, nsIURI*, mozilla::Maybe<nsCOMPtr<nsIURI> > const&, bool, bool, nsIURI*, bool, unsigned int, nsIPrincipal*, nsIPrincipal*, char const*, nsTSubstring<char16_t> const&, nsIInputStream*, long, nsIInputStream*, bool, nsIDocShell**, nsIRequest**, bool, bool, bool, nsTSubstring<char16_t> const&, nsIURI*, unsigned int) (/home/jld/src/gecko-dev/docshell/base/nsDocShell.cpp:11532)
Sandbox: frame #27: nsDocShell::InternalLoad(nsIURI*, nsIURI*, mozilla::Maybe<nsCOMPtr<nsIURI> > const&, bool, nsIURI*, unsigned int, nsIPrincipal*, nsIPrincipal*, unsigned int, nsTSubstring<char16_t> const&, char const*, nsTSubstring<char16_t> const&, nsIInputStream*, long, nsIInputStream*, unsigned int, nsISHEntry*, bool, nsTSubstring<char16_t> const&, nsIDocShell*, nsIURI*, bool, nsIDocShell**, nsIRequest**) (/home/jld/src/gecko-dev/docshell/base/nsDocShell.cpp:10875)
--DOMWINDOW == 0 (0x7f42c7d52000) [pid = 58708] [serial = 4] [outer = (nil)] [url = about:home]
Sandbox: frame #28: nsDocShell::LoadURI(nsIURI*, nsIDocShellLoadInfo*, unsigned int, bool) (/home/jld/src/gecko-dev/docshell/base/nsDocShell.cpp:1635)
Assignee | ||
Comment 2•7 years ago
|
||
And there's another class of failure, from the Web Platform Tests, which doesn't involve file:/// content-type guessing but instead has something to do with external protocol handlers. You might think that bug 1382323 would take care of that, but no. I can't reproduce these locally, so I can't get precise unwinding, and there's some weirdness in the unwinding on Try, like showing HandlerServiceParent being called in a content process[1]. Another stack[2] looks slightly more reasonable:
16 libxul.so!nsGIOService::GetAppForURIScheme [nsGIOService.cpp:597a56d5697f : 275 + 0x9]
17 libxul.so!nsGNOMERegistry::GetAppDescForScheme [nsGNOMERegistry.cpp:597a56d5697f : 49 + 0xc]
20 libxul.so!nsOSHelperAppService::GetApplicationDescription [nsOSHelperAppService.cpp:597a56d5697f : 1154 + 0x5]
21 libxul.so!nsOSHelperAppService::GetProtocolHandlerInfoFromOS [nsOSHelperAppService.cpp:597a56d5697f : 1524 + 0x3]
30 libxul.so!nsExternalHelperAppService::GetProtocolHandlerInfo [nsExternalHelperAppService.cpp:597a56d5697f : 1101 + 0x9]
31 libxul.so!nsExternalHelperAppService::ExternalProtocolHandlerExists [nsExternalHelperAppService.cpp:597a56d5697f : 905 + 0x11]
32 libxul.so!nsExternalProtocolHandler::HaveExternalProtocolHandler [nsExternalProtocolHandler.cpp:597a56d5697f : 527 + 0xe]
33 libxul.so!nsExternalProtocolHandler::NewChannel2
34 libxul.so!mozilla::net::nsIOService::NewChannelFromURIWithProxyFlagsInternal
40 libxul.so!NS_NewChannelInternal
41 libxul.so!mozilla::net::HttpChannelChild::SetupRedirect
[1] https://treeherder.mozilla.org/logviewer.html#?job_id=136905959&repo=try&lineNumber=6354
[2] https://treeherder.mozilla.org/logviewer.html#?job_id=136905336&repo=try&lineNumber=1902
Comment 3•7 years ago
|
||
The 2nd stack makes sense. ExternalProtocolHandlerExists will first try GetPossibleApplicationHandlers, calling into GIO, and if that fails, fall back to OSProtocolHandlerExists - which is now remoted. But this does means it will first try to do that first call in content context. Bummer.
Updated•7 years ago
|
Priority: -- → P3
Updated•7 years ago
|
Priority: P3 → P2
Assignee | ||
Comment 4•7 years ago
|
||
So the case in comment #2 will “just work” if we return an error from inotify_init, although in that case it's possible that glib would do something reasonable internally and not return an error into the XPCOM stuff.
Comment #1… it looks like it has an mtime-based fallback (g_app_info_get_*_for_type -> get_all_desktop_entries_for_mime_type -> mime_info_cache_init -> mime_info_cache_update_dir_lists), so that might also work well enough.
I'm going to take this bug on the assumption that I can just ENOSYS those and don't need to rewire the helper app service.
Assignee: nobody → jld
Assignee | ||
Comment 5•7 years ago
|
||
Looking at the code some more, it's possible that failing inotify_init would make GIO fall back to FAM, if it was configured with it, and that could cause it to try to do networking. I don't know how common that configuration is; also I haven't tested this so I don't even know if that's what would happen. (Also, if I'm right, it will use FAM anyway for NFS… but see also bug 1409900, which I'm commenting on concurrently.)
It might be possible to use g_io_extension_point_implement to register a dummy file monitor component that won't do anything, but that seems kind of excessive given that *we control the code that is calling GIO in the first place*, and it really should not ever be calling this stuff in a content process.
I could also just make nsGIOService and/or nsGNOMERegistry fail to initialize in content processes, like I did to GConf; not sure how much collateral damage there'd be.
Assignee | ||
Comment 6•7 years ago
|
||
If we just return failure for socket() instead of crashing on Nightly, then FAM isn't a problem. And we'd have to do that anyway unless we make some changes in WebRTC. So I'm thinking I'll just soft-fail inotify_init, and when we block socket() it will just return an error (like we did on B2G) and forbidding it entirely can be a followup.
Comment hidden (mozreview-request) |
Updated•7 years ago
|
Priority: P2 → P1
Whiteboard: sb+
Comment 8•7 years ago
|
||
mozreview-review |
Comment on attachment 8922170 [details]
Bug 1408497 - Disallow inotify in sandboxed content processes.
https://reviewboard.mozilla.org/r/193200/#review198698
Attachment #8922170 -
Flags: review?(gpascutto) → review+
Pushed by jedavis@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/4381412e49e3
Disallow inotify in sandboxed content processes. r=gcp
Comment 10•7 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in
before you can comment on or make changes to this bug.
Description
•