Closed Bug 1422456 (CVE-2018-12401) Opened 4 years ago Closed 3 years ago
about:home crashes whole browser (possible memory corruption)
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/62.0.3202.94 Safari/537.36 Steps to reproduce: Visit the following URL: about:home?1:q "MOZ_CRASH(Should never get here!)" is the reason. Actual results: Report: https://crash-stats.mozilla.com/report/index/5bd57607-cb08-4606-b085-9755d0171202#tab-details Expected results: No crash. Also I will mark as sensitive since I am seeing the words "MEMORY_CORRUPTION_LARGE_80000003_memory_corruption!KERNEL32" in my windbg, though I'm not sure if it is. Also, about:home is linkable from web content, so if this was exploitable its even more dangerous as it requires no user interaction.
Quota manager -> baku, can you take a look?
janv knows more about QM.
Flags: needinfo?(amarchesini) → needinfo?(jvarga)
Group: firefox-core-security → dom-core-security
Component: Untriaged → DOM: IndexedDB
Product: Firefox → Core
FWIW this is the windbg crash dump that contains 'MEMORY_CORRUPTION_LARGE_80000003_memory_corruption!KERNEL32'
Crashes the whole browser for me -- boom down in IndexedDB bp-90668379-16a2-4fbf-8e83-32b7e0180419 Could not reproduce in 58, 59 or 60 with a fresh profile which is kind of odd. Maybe I didn't have enough stored in IndexedDB for it to need to check the per-site quota? Haven't yet tried a fresh profile in nightly. Doesn't seem exploitable in the malware sense, just a DoS -- but a bad one because it kills the parent process and not just the tab.
Assignee: nobody → jvarga
Status: NEW → ASSIGNED
I can verify current Nightly hits the assertion here  in debug build. If I comment , then it will crash in , which looks like the place comment #0 mentioned. (Somehow I couldn't see the crash report)  https://searchfox.org/mozilla-central/rev/de7676288a78b70d2b9927c79493adbf294faad5/dom/quota/ActorsParent.cpp#8637  https://searchfox.org/mozilla-central/rev/de7676288a78b70d2b9927c79493adbf294faad5/dom/quota/ActorsParent.cpp#8723
I checked the origin  from GetInfoFromPrincipal() if typing "about:home?1:q". I expected that origin should be "about:home", but, actually, I got "about:home?1:q". It seems that about protocol doesn't strip the ref and query which isn't expected by QuotaManager. So that if typing "about:home?1" or "about:home#2", I will get "about+home+1", and "about+home+2" in my profile. I reckon in this bug we should do things below: 1. Make origin parser not be so strict. At least, it shouldn't let FF crash  if the format of handling protocol isn't as expected. We may still want to capture issues on Nightly via crashing it. But, as for Release and Beta, just return false should be fine. 2. Strip '?xxx' or '#xxx' for about protocol in GetInfoFromPrincipal() or returning false earlier  if mSchemeType is eNone in OriginParser.  https://searchfox.org/mozilla-central/rev/de7676288a78b70d2b9927c79493adbf294faad5/dom/quota/ActorsParent.cpp#5613  https://searchfox.org/mozilla-central/rev/de7676288a78b70d2b9927c79493adbf294faad5/dom/quota/ActorsParent.cpp#8723  https://searchfox.org/mozilla-central/rev/de7676288a78b70d2b9927c79493adbf294faad5/dom/quota/ActorsParent.cpp#8636-8663
(In reply to Tom Tung [:tt] from comment #6) > I checked the origin  from GetInfoFromPrincipal() if typing > "about:home?1:q". I expected that origin should be "about:home", but, > actually, I got "about:home?1:q". According to the nsIPrincipal's interface  and the MDN documents for origin  (also the reference documents inside it), I expected the origin which getting from the Principal object shouldn't contain '?'. However, "about:home" seems to be an internal implementation for Gecko, so I'm not really sure should I fix internally in dom/quota/ActorsParent.cpp or somewhere else when the principal object for "about:xxx" is created. baku, do you have any suggestion about that?  https://searchfox.org/mozilla-central/source/caps/nsIPrincipal.idl#220-226  https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Origin
Here a patch to fix the origin for about/moz-safe-about/indexeddb URLs.
Comment on attachment 9008795 [details] [diff] [review] origin_about.patch We must get tests for this. As of now, easiest might be some browser test which loads about:robots?foo and its contentDocument.nodePrincipal.URI.spec should be then about:robots. Need to test also # url with both # and ?. I'd like to review this with the test. Can the crash be somehow triggered by web page?
(In reply to Olli Pettay [:smaug] from comment #9) > Can the crash be somehow triggered by web page? At the time of the report, yes. But ever since about:home has become unlinkable from web, so as of now, no.
Tom, so the URL fix has landed, but are we sure that there can't be any origin directories like about+home+1 ? Maybe such directories could have been created, in that case we need to extend origin parser to support it.
I open bug 1491637 for discussing and investigating that if they were created, we might want to support them in that bug.
Is this worth backporting to Beta? If so, go ahead and do the request, it grafts cleanly.
Comment on attachment 9008853 [details] [diff] [review] origin_about.patch Approval Request Comment [Feature/Bug causing the regression]: origin computation for about: URLs [User impact if declined]: ff crashes [Is this code covered by automated tests?]: yes [Has the fix been verified in Nightly?]: yes [Needs manual test from QE? If yes, steps to reproduce]: yes, see the description of the bug [List of other uplifts needed for the feature/fix]: none [Is the change risky?]: no [Why is the change risky/not risky?]: The fix consists in the removal of the ref and query part of about: URLs, exactly how we do for http/https/ftp/.. URLs. [String changes made/needed]: none
Attachment #9008853 - Flags: approval-mozilla-beta?
Comment on attachment 9008853 [details] [diff] [review] origin_about.patch Crash fix verified on Nightly, approved for 63 Beta 8, thanks.
Attachment #9008853 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Reproduced this crash on Windows 10x64 using Fx 63.0b1, buildID 20180824192747. Confirmed the fix on latest Nightly, build ID: 20180919003800. Leaving the qe+ flag in place until I`ll also cover this fix on Beta 63.
Also confirming the fix for Fx 63.0b8 (buildID: 20180920135444).
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main63+]
You need to log in before you can comment on or make changes to this bug.