Closed Bug 1342552 Opened 3 years ago Closed 2 years ago

Crash in nsViewManager::GetRootWidget

Categories

(Core :: DOM: Events, defect, critical)

51 Branch
defect
Not set
critical

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox-esr45 --- wontfix
firefox51 --- wontfix
firefox52 --- wontfix
firefox-esr52 54+ fixed
firefox53 --- wontfix
firefox54 + fixed
firefox55 + fixed

People

(Reporter: philipp, Assigned: masayuki)

References

Details

(4 keywords, Whiteboard: [post-critsmash-triage][adv-main54+][adv-esr52.2+])

Crash Data

Attachments

(2 files, 3 obsolete files)

This bug was filed from the Socorro interface and is 
report bp-fc606b00-f700-442e-af4c-b9aa62170224.
=============================================================
Crashing Thread (0)
Frame 	Module 	Signature 	Source
0 	xul.dll 	nsViewManager::GetRootWidget(nsIWidget**) 	view/nsViewManager.cpp:1072
1 	xul.dll 	nsPresContext::GetRootWidget() 	layout/base/nsPresContext.cpp:1116
2 	xul.dll 	mozilla::IMEStateManager::OnChangeFocusInternal(nsPresContext*, nsIContent*, mozilla::widget::InputContextAction) 	dom/events/IMEStateManager.cpp:403
3 	xul.dll 	mozilla::IMEStateManager::OnChangeFocus(nsPresContext*, nsIContent*, mozilla::widget::InputContextAction::Cause) 	dom/events/IMEStateManager.cpp:375
4 	xul.dll 	nsFocusManager::Focus(nsPIDOMWindowOuter*, nsIContent*, unsigned int, bool, bool, bool, bool, nsIContent*) 	dom/base/nsFocusManager.cpp:1958
5 	xul.dll 	nsFocusManager::WindowShown(mozIDOMWindowProxy*, bool) 	dom/base/nsFocusManager.cpp:910
6 	xul.dll 	nsGlobalWindow::SetReadyForFocus() 	dom/base/nsGlobalWindow.cpp:10164
7 	xul.dll 	nsGlobalWindow::SetReadyForFocus() 	dom/base/nsGlobalWindow.cpp:10157
8 	xul.dll 	PresShell::UnsuppressAndInvalidate() 	layout/base/nsPresShell.cpp:3820
9 	xul.dll 	PresShell::sPaintSuppressionCallback(nsITimer*, void*) 	layout/base/nsPresShell.cpp:1808
10 	xul.dll 	nsTimerImpl::Fire() 	xpcom/threads/nsTimerImpl.cpp:521
11 	xul.dll 	nsTimerEvent::Run() 	xpcom/threads/TimerThread.cpp:286
12 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp:1067
13 	xul.dll 	mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp:96
14 	xul.dll 	mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp:301
15 	xul.dll 	_SEH_epilog4

this cross-platform crash signature has been around for a while. many user comments indicate they were printing something at the time of the crash or hitting the back button to navigate to a prior site.

Correlations for Firefox Beta
(77.42% in signature vs 01.61% overall) Module "prnfldr.dll" = true [100.0% vs 02.32% if platform_pretty_version = Windows 7]
(96.77% in signature vs 28.84% overall) Module "winspool.drv" = true [100.0% vs 14.54% if platform_version = 6.1.7600]
(90.32% in signature vs 00.12% overall) address = 0xffffffffe5e5e5f1
(96.77% in signature vs 33.31% overall) Module "comdlg32.dll" = true [100.0% vs 18.48% if platform_version = 6.1.7600]
(100.0% in signature vs 33.85% overall) reason = EXCEPTION_ACCESS_VIOLATION_READ
(98.39% in signature vs 31.91% overall) top(none)/detached > 0 = null

(marking this as security sensitive due to the uaf situation)
Can we tell if there's a correlation in which IME people are using? or is it all in our code and independent of that?
Component: General → DOM: Events
Flags: needinfo?(masayuki)
Keywords: csectype-uaf
I don't think that this is related to IME.

Looks like that before firing the timer, the relation between PresShell, ViewManager and nsIWidget has been broken.

# FYI: If it's crashed in content process, we cannot know which IME is used at crash due to not loaded any DLL of IME in content process.
Flags: needinfo?(masayuki)
This is during shutting down, so prescontext or presshell is already destroyed?
ah, if not shutting down, But WindowShown is called, but nsViewManager is invalid. why...
When nsView is destroyed, it removes itself from nsViewManager:
https://dxr.mozilla.org/mozilla-central/rev/d29f84406483c721a13cf9a52936ecced0c5c98a/view/nsView.cpp#80,84,94,96-97

nsView::mViewManager is set by its constructor and never set to nullptr until destructor runs. So, the view can be root view after desctructor of nsView is run? The destructor is called from nsView::Destroy():
https://dxr.mozilla.org/mozilla-central/rev/d29f84406483c721a13cf9a52936ecced0c5c98a/view/nsView.cpp#184,186
nsViewManager::SetRootView() is called by PresShell, nsDocumentView and nsPrintEngine:
https://dxr.mozilla.org/mozilla-central/search?q=%2Bcallers%3A%22nsViewManager%3A%3ASetRootView%28nsView+%2A%29%22

The former 2 call it with the result of nsViewManager::CreateView().  On the other hand, nsPrintEngine may not do so in some cases.
If we could stop handling focus with detecting the destroy, perhaps, it should be stopped in nsFocusManager.
Group: core-security → dom-core-security
(In reply to Masayuki Nakano [:masayuki] from comment #7)
> If we could stop handling focus with detecting the destroy, perhaps, it
> should be stopped in nsFocusManager.

Is that something you could handle or would you like me to try to find someone else to take it?
Flags: needinfo?(masayuki)
I think that it's better somebody who is really familiar with the relation between view and those 3 classes to take this bug. I cannot say "must be fixed with this patch" without STR...
Flags: needinfo?(masayuki)
Okay, looks like that nsPrintEngine has a bug. nsPrintEngine::SetRootView() also crashes at setting root view:
https://crash-stats.mozilla.com/report/index/45e8c9a4-eb2a-472b-a88e-be90d2170227
> 0 	xul.dll 	nsPrintEngine::SetRootView(nsPrintObject*, bool&, bool&, nsSize&) 	layout/printing/nsPrintEngine.cpp:2102
> 1 	xul.dll 	nsPrintEngine::ReflowPrintObject(nsPrintObject*) 	layout/printing/nsPrintEngine.cpp:2165
> 2 	xul.dll 	nsPrintEngine::ReflowDocList(nsPrintObject*, bool) 	layout/printing/nsPrintEngine.cpp:1798

https://hg.mozilla.org/releases/mozilla-release/annotate/327e081221b0/layout/printing/nsPrintEngine.cpp#l2102
Olli, can you please take a look here?
Flags: needinfo?(bugs)
Too late for firefox 52, mass-wontfix.
Although, I'm still not sure the actual reason of using released nsView.

If PresShell or DocShell is being destroyed properly in such case, my experimental patch could "hide" this bug.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=6e7b9149c5c16ce80f046efb0640afb19d5ad44f

I'm not sure if I should file another bug as just a cleanup code bug or something for it.
My random thoughts:

* nsView isn't refcountable, it's destroyed when its owner frame is destroyed, it's a root view of an nsViewManager and it's being destroyed, or its parent whose nsViewManager is same and is being destroyed.
* So, nsView pointer can be dirty easily.  If it's possible, it should be managed with |UniquePtr<nsView>| and referenced by |const UniquePtr<nsView>&|.
* It might be possible nsViewManager to store outdated nsView pointer as its root if SetRootView() is called with outdated pointer.
* Non-fresh nsView instance is set only from nsPrintEngine::SetRootView(): http://searchfox.org/mozilla-central/rev/381a7b8f8a78b481336cfa384cb0a5536e217e4a/layout/printing/nsPrintEngine.cpp#2084,2086,2102
* The parent view might be outdated: http://searchfox.org/mozilla-central/rev/381a7b8f8a78b481336cfa384cb0a5536e217e4a/layout/printing/nsPrintEngine.cpp#2081,2089,2093
* nsPrintEngine::GetParentViewForRoot() returns the result of nsIContentViewer::FindContainerViewer(): http://searchfox.org/mozilla-central/rev/381a7b8f8a78b481336cfa384cb0a5536e217e4a/layout/printing/nsPrintEngine.cpp#2024,2026-2027,2029
* nsIContentViewer::FindContainerViewer() returns a child view of its view: http://searchfox.org/mozilla-central/rev/381a7b8f8a78b481336cfa384cb0a5536e217e4a/layout/base/nsDocumentViewer.cpp#2522,2557 http://searchfox.org/mozilla-central/rev/381a7b8f8a78b481336cfa384cb0a5536e217e4a/layout/generic/nsSubDocumentFrame.cpp#1206,1208-1209,1213,1218,1223,1226
* However, nsSubDocumentFrame::mInnerView isn't cleared when its parent (mOuterView) is destroyed. On the other hand, it's a frame class, so, when mOuterView is destroyed, the instance should also be deleted...
* On the other hand, nsPrintEngine::ReconstructAndReflow() is scared. It refers cached nsPrintObject in for loop and also flushes pending notifications! http://searchfox.org/mozilla-central/rev/381a7b8f8a78b481336cfa384cb0a5536e217e4a/layout/printing/nsPrintEngine.cpp#1589-1590,1614,1623
* So, mPrintDocList should be nsTArray<const UniquePtr<nsPrintObject>&>, instead of nsTArray<nsPrintObject*>.


FYI: I tried to catch the case when nsPrintEngine::SetRootView() reuses the root view but PresShell has gone yesterday. But I couldn't be hit it...
I'm not sure if this is possible case. Enn, how about you?
Attachment #8854721 - Flags: feedback?(enndeakin)
Comment on attachment 8854721 [details] [diff] [review]
Focus handler doesn't need to try to move focus if new focused document is being destroyed

Seems ok.
Attachment #8854721 - Flags: feedback?(enndeakin) → feedback+
Flags: needinfo?(bugs)
Assignee: nobody → masayuki
Status: NEW → ASSIGNED
Comment on attachment 8854721 [details] [diff] [review]
Focus handler doesn't need to try to move focus if new focused document is being destroyed

I'm not sure who is most familiar with this.

But, mstange is the module owner of view system.
https://wiki.mozilla.org/Modules/Core#View_System

See also comment 16. I'm still not sure why nsViewManager can have outdated nsView pointer as its root view. If you have an idea of that, let me know.
Attachment #8854721 - Flags: review?(mstange)
Attachment #8854721 - Flags: review?(mstange) → review?(tnikkel)
Ah, the patch's comment is same as at testing it on tryserver.

It should be:

> Bug 1342552 Don't try to move focus if the PresShell has gone
> 
> If focus is moving to an element which is in unloading document, should stop moving focus in either nsFocusManager or IMEStateManager because setting focus to such element doesn't make sense.

I'll update it after review before requesting sec-approval.


BTW, due to not sure if the patch gets to fix this bug, I think that it's better to file a normal bug to clean up the focus movement and land the patch normally because somebody could search the security bug from the landed patch but it may not be fixed even with this patch. Then, they might find the STR and some ways to attach with it.
Ah, but even if we'd do so, uplifting this kind of patch is odd for the watchers, sigh...
Filed bug 1354443 for the life time issue of printing objects.
Comment on attachment 8854721 [details] [diff] [review]
Focus handler doesn't need to try to move focus if new focused document is being destroyed

>+  // If new presShell has been destroyed, this should handle as that nobody
>+  // is getting focus.
>+  if (aPresContext &&
>+      (NS_WARN_IF(!aPresContext->GetPresShell()) ||
>+       NS_WARN_IF(aPresContext->GetPresShell()->IsDestroying()))) {
>+    MOZ_LOG(sISMLog, LogLevel::Warning,
>+      ("  OnChangeFocusInternal(), called with destroyed presShell, "
>+       "handling this call as nobody getting focus"));
>+    aPresContext = nullptr;
>+    aContent = nullptr;
>+  }
>+
>   bool focusActuallyChanging =
>     (sContent != aContent || sPresContext != aPresContext ||
>      sActiveTabParent != newTabParent);
> 
>   nsCOMPtr<nsIWidget> oldWidget =
>     sPresContext ? sPresContext->GetRootWidget() : nullptr;
>   if (oldWidget && focusActuallyChanging) {
>     // If we're deactivating, we shouldn't commit composition forcibly because

Aren't we crashing when accessing sPresContext? Not aPresContext? So how would this help?

The patch seems fine as an attempt to fix this crash without knowing the root cause.
(In reply to Masayuki Nakano [:masayuki] from comment #23)
> Filed bug 1354443 for the life time issue of printing objects.

Would you mind cc'ing me on that?
(In reply to Timothy Nikkel (:tnikkel) from comment #25)
> (In reply to Masayuki Nakano [:masayuki] from comment #23)
> > Filed bug 1354443 for the life time issue of printing objects.
> 
> Would you mind cc'ing me on that?

No, I added you.

(In reply to Timothy Nikkel (:tnikkel) from comment #24)
> Comment on attachment 8854721 [details] [diff] [review]
> Focus handler doesn't need to try to move focus if new focused document is
> being destroyed
> 
> >+  // If new presShell has been destroyed, this should handle as that nobody
> >+  // is getting focus.
> >+  if (aPresContext &&
> >+      (NS_WARN_IF(!aPresContext->GetPresShell()) ||
> >+       NS_WARN_IF(aPresContext->GetPresShell()->IsDestroying()))) {
> >+    MOZ_LOG(sISMLog, LogLevel::Warning,
> >+      ("  OnChangeFocusInternal(), called with destroyed presShell, "
> >+       "handling this call as nobody getting focus"));
> >+    aPresContext = nullptr;
> >+    aContent = nullptr;
> >+  }
> >+
> >   bool focusActuallyChanging =
> >     (sContent != aContent || sPresContext != aPresContext ||
> >      sActiveTabParent != newTabParent);
> > 
> >   nsCOMPtr<nsIWidget> oldWidget =
> >     sPresContext ? sPresContext->GetRootWidget() : nullptr;
> >   if (oldWidget && focusActuallyChanging) {
> >     // If we're deactivating, we shouldn't commit composition forcibly because
> 
> Aren't we crashing when accessing sPresContext? Not aPresContext? So how
> would this help?

Ah, good point. It's just my mistake.

> The patch seems fine as an attempt to fix this crash without knowing the
> root cause.

Thanks. Although, we should find the root cause in the other bugs.
So were you going to update the patch to remove that bit? Or to check sPresContext instead?
Flags: needinfo?(masayuki)
(In reply to Timothy Nikkel (:tnikkel) from comment #27)
> So were you going to update the patch to remove that bit? Or to check
> sPresContext instead?

Yeah, I was. But I realized that we can take safer patch here.

If this occurs only with sPresContext->GetRootWidget(), we can cache the widget when it causes sPresContext. I think that such patch could be perfect wallpaper for this crash since GetRootWidget() won't be used with sPresContext.
Flags: needinfo?(masayuki)
Comment on attachment 8854721 [details] [diff] [review]
Focus handler doesn't need to try to move focus if new focused document is being destroyed

Clearing review in anticipation of new patch.
Attachment #8854721 - Flags: review?(tnikkel)
Sorry for the delay due to avoiding existing bug of IMEStateManager::UpdateIMEState() callers.

This patch makes IMEStateManager use GetRootWidget() of sPresContext and destroyed PresContext.  So, this must be perfect wallpaper for this crash even though this doesn't fix the actual bug around nsView or printing objects lifetiem management.

This patch looks big, but the changes are simple. Using IMEStateManager::sWidget when sPresContext::GetRootWidget() is necessary.

Other changes are:
* Avoiding the bug of UpdateIMEState() callers. They shouldn't call it before IMEStateManager::OnFocusChange(), but they do it in some cases. I'll file follow up bug for it.
* Using nsCOMPtr<nsIWidget> before calling some methods of nsIWidget.
Attachment #8854721 - Attachment is obsolete: true
Attachment #8857973 - Flags: review?(tnikkel)
The patch is all in IMEStateManager, which I've never looked at before. Is there a better reviewer?
Flags: needinfo?(masayuki)
Okay, I'll ask its review to smaug.
Flags: needinfo?(masayuki)
Attachment #8857973 - Flags: review?(tnikkel) → review?(bugs)
Comment on attachment 8857973 [details] [diff] [review]
IMEStateManager should cache nsIWidget for sPresContext and use it

I'm worried that nothing keeps sWidget alive when it is being used.
Shouldn't we take a strong reference to it before using it?

Ensuring that sWidget never dies at unexpected time is really hard from this patch. And there even are checks that it may die during some calls
+    NotifyIME(REQUEST_TO_COMMIT_COMPOSITION, sWidget);
+    if (NS_WARN_IF(!sWidget)) {

So, looks rather scary.

Strong reference before using sWidget and then perhaps some flag in nsIWidget telling whether it has been destroyed? (nsBaseWidget::Destroy())
Attachment #8857973 - Flags: review?(bugs) → review-
Thanks.

And I changed that IMEStateManager uses nsCOMPtr<nsIWidget> widget(sWidget); even before calling nsIWidget::GetInputContext() because it does not change anything in normal cases.  However, it may call Windows API. Then, other application can do anything if they use API hook. So, it could cause destroying the widget (although, it shouldn't occur in the real world).
Attachment #8857973 - Attachment is obsolete: true
Attachment #8859542 - Flags: review?(bugs)
Comment on attachment 8859542 [details] [diff] [review]
IMEStateManager should cache nsIWidget for sPresContext and use it

>+  // If new aPresShell has been destroyed, this should handle the focus change
>+  // as nobody getting focus.
as nobody is getting focus

>+  // If it's an HTML editor, nsIEditor::PostCreate() may be called before
>+  // a call of IMEStateManager::OnFocusChange().  If it's a plain text editor,
>+  // nsIEditor::SetFlags() may becalled before a call of it too.  In such
Don't quite understand the sentence.
"may be called before" something (I'm not sure what). Clarify this a bit.
>   // This method updates the current IME state.  However, if the enabled state
>   // isn't changed by the new state, this method does nothing.
>   // Note that this method changes the IME state of the active element in the
>   // widget.  So, the caller must have focus.
>   static void UpdateIMEState(const IMEState &aNewIMEState,
>                              nsIContent* aContent,
>-                             nsIEditor* aEditor);
>+                             EditorBase& aEditorBase);
It is unclear to me why & and not *.
But shouldn't matter much if it is guaranteed that aEditorBase stays alive during the method call.
> 
>+  /**
>+   * CanHandleWith() returns false if aPresContext is nullptr or it's destroyed.
>+   */
>+  static bool CanHandleWith(nsPresContext* aPresContext);

ok, so it deal with null.
Then the explicit null check can be removed in places like
>+  if (NS_WARN_IF(!aPresContext) ||
>+      NS_WARN_IF(!CanHandleWith(aPresContext))) {


>+
>+  // sContent and sPresContext are the focused content and presShell. 
Certainly aPresContext isn't a presshell
Attachment #8859542 - Flags: review?(bugs) → review+
(In reply to Olli Pettay [:smaug] from comment #35)
> >+  // If it's an HTML editor, nsIEditor::PostCreate() may be called before
> >+  // a call of IMEStateManager::OnFocusChange().  If it's a plain text editor,
> >+  // nsIEditor::SetFlags() may becalled before a call of it too.  In such
> Don't quite understand the sentence.
> "may be called before" something (I'm not sure what). Clarify this a bit.

Perhaps, this should be clearer:

  // IMEStateManager::UpdateIMEState() should be called after
  // IMEStateManager::OnChangeFocus() is called for setting focus to aContent
  // and aEditorBase.  However, when aEditorBase is an HTMLEditor, this may be
  // called by nsIEditor::PostCreate() before IMEStateManager::OnChangeFocus().
  // Similarly, when aEditorBase is a TextEditor, this may be called by
  // nsIEditor::SetFlags().  In such cases, this method should do nothing
  // because input context should be updated when
  // IMEStateManager::OnChangeFocus() is called later.

> >   // This method updates the current IME state.  However, if the enabled state
> >   // isn't changed by the new state, this method does nothing.
> >   // Note that this method changes the IME state of the active element in the
> >   // widget.  So, the caller must have focus.
> >   static void UpdateIMEState(const IMEState &aNewIMEState,
> >                              nsIContent* aContent,
> >-                             nsIEditor* aEditor);
> >+                             EditorBase& aEditorBase);
> It is unclear to me why & and not *.
> But shouldn't matter much if it is guaranteed that aEditorBase stays alive
> during the method call.

Yes, the callers are methods of nsIEditor. So, their callers should guarantee the editor instance won't be destroyed.

> >+  /**
> >+   * CanHandleWith() returns false if aPresContext is nullptr or it's destroyed.
> >+   */
> >+  static bool CanHandleWith(nsPresContext* aPresContext);
> 
> ok, so it deal with null.
> Then the explicit null check can be removed in places like
> >+  if (NS_WARN_IF(!aPresContext) ||
> >+      NS_WARN_IF(!CanHandleWith(aPresContext))) {

Ah, good point! Thanks!
[Security approval request comment]
How easily could an exploit be constructed based on the patch?

I don't know because we have no idea to reproduce this crash.

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

No. This patch does NOT fix the root problem of the crash (deleted nsView use). This patch completely avoids to use nsView of cached PresContext with caching widget for PresContext directly.

Which older supported branches are affected by this flaw?

Not sure, but not recent regression.

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

This patch can be applied to Nightly, Aurora and Beta. I'll attach a patch for ESR52.

How likely is this patch to cause regressions; how much testing does it need?

Not so risky even though this patch looks big. This patch avoids to use PresContext::GetRootWidget() with cached PresContext and makes IMEStateManager caches root widget for focused PresContext. Therefore, this patch modifies a lot of lines to use the cache, but the changes are simple.
Attachment #8859542 - Attachment is obsolete: true
Attachment #8859910 - Flags: sec-approval?
Sec-approval+ for checkin on *May 3* or later, two weeks into the new cycle. We'll want this back ported so please nominate for all affected branches as well.
Whiteboard: [checkin on 5/3]
Attachment #8859910 - Flags: sec-approval? → sec-approval+
[Tracking Requested - why for this release]:
Same crash reports come from any active branches including ESR45.

I don't think that we should land this patch into ESR45 due to too big and becoming EOL (IIRC). However, it should be decided by security team. If it's necessary for ESR45 too. I'll prepare to write the patch for ESR45 too.

For the other branches, we should take it because this is sec-high.
(In reply to Al Billings [:abillings] from comment #39)
> Sec-approval+ for checkin on *May 3* or later, two weeks into the new cycle.
> We'll want this back ported so please nominate for all affected branches as
> well.

Oh, I realized that the first week of May is a holiday week of Japan. So, I'd like somebody to land it on May 3.
Track 54+/55+ as sec-high.
https://hg.mozilla.org/integration/mozilla-inbound/rev/7e75e46b355b9dbdfb2d3fea9a4f074dd3143d39

There are no further planned ESR45 releases, so setting that branch to wontfix.
Whiteboard: [checkin on 5/3]
Comment on attachment 8859910 [details] [diff] [review]
IMEStateManager should cache nsIWidget for sPresContext and use it

Approval Request Comment
[Feature/Bug causing the regression]: unknown, but very old
[User impact if declined]: exploitable crashes
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: no
[Needs manual test from QE? If yes, steps to reproduce]: no
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: not very
[Why is the change risky/not risky?]: per comment 37, it's a big patch but the changes are mechanical and the change made is straightforward
[String changes made/needed]: none

I've confirmed that this grafts cleanly to Beta.
Attachment #8859910 - Flags: approval-mozilla-beta?
Attachment #8859911 - Flags: approval-mozilla-esr52?
https://hg.mozilla.org/mozilla-central/rev/7e75e46b355b
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Group: dom-core-security → core-security-release
Comment on attachment 8859910 [details] [diff] [review]
IMEStateManager should cache nsIWidget for sPresContext and use it

Fix a sec-high. Beta54+ & ESR52+. Should be in 54 beta 6.
Attachment #8859910 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8859911 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52+
Flags: qe-verify-
Whiteboard: [post-critsmash-triage]
Whiteboard: [post-critsmash-triage] → [post-critsmash-triage][adv-main54+][adv-esr52.2+]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.