Closed Bug 1669466 (CVE-2020-26959) Opened 4 years ago Closed 4 years ago

Heap use-after-free via ContentParent::RecvInitStreamFilter

Categories

(WebExtensions :: Request Handling, defect, P2)

Product:
WebExtensions
Component:
Request Handling
Type:
defect

Tracking

(firefox-esr7883+ fixed, firefox82 wontfix, firefox83 fixed, firefox84+ fixed)

Status:
RESOLVED FIXED
Milestone:
84 Branch
Tracking Flags:
Tracking Status
firefox-esr78 83+ fixed
firefox82 --- wontfix
firefox83 --- fixed
firefox84 + fixed

People

(Reporter: tunnelshade, Assigned: rpl)

Details

(Keywords: csectype-uaf, reporter-external, sec-moderate, Whiteboard: [reporter-external] [client-bounty-form] [verif?][adv-main83+][adv-esr78.5+])

Attachments

(4 files)

latest-stock-85713
76 bytes, application/octet-stream
Details
600f47bbfeb2b8dd8feb52dc9b0df0c72e01da9e-m-c
76 bytes, application/octet-stream
Details
Bug 1669466 - Change WebRequestService singleton to a StaticRefPtr. r=glandium!,mixedpuppy!
47 bytes, text/x-phabricator-request
jcristau
: approval-mozilla-beta+
jcristau
: approval-mozilla-esr78+
Details | Review
advisory.txt
243 bytes, text/plain
Details
Attached file latest-stock-85713 —

Introduction

While fuzzing ContentParent IPC with a slightly modified harness, I came across a very weird crash. Crash happens during xpcom shutdown, possibly due to refcounting a freed instance via a stale pointer.

Steps

Used fuzzfetch to get latest release fuzzing build to reproduce on stock harness.

$ ./m-r-20201005120340-fuzzing-asan-opt/firefox latest-stock-85713

*** You are running in headless mode.
Running Fuzzer tests...
INFO: Seed: 1293591897
INFO: Loaded 1 modules   (1524233 inline 8-bit counters): 1524233 [0x7f76db165038, 0x7f76db2d9241),
INFO: Loaded 1 PC tables (1524233 PCs): 1524233 [0x7f76db2d9248,0x7f76dca1b2d8),
./m-r-20201005120340-fuzzing-asan-opt/firefox: Running 1 inputs 1 time(s) each.
Running: triage-2/latest-stock-85713
Executed triage-2/latest-stock-85713 in 0 ms
***
*** NOTE: fuzzing was not performed, you have only
***       executed the target code on a fixed set of inputs.
***
=================================================================
==560090==ERROR: AddressSanitizer: heap-use-after-free on address 0x604000092a50 at pc 0x7f76d1282502 bp 0x7ffc50927ce0 sp 0x7ffc50927cd8
READ of size 8 at 0x604000092a50 thread T0 (MainThread)
    #0 0x7f76d1282501 in operator-- /builds/worker/workspace/obj-build/dist/include/nsISupportsImpl.h:321:34
    #1 0x7f76d1282501 in Release /builds/worker/checkouts/gecko/toolkit/components/extensions/webrequest/WebRequestService.h:46:3
    #2 0x7f76d1282501 in Release /builds/worker/workspace/obj-build/dist/include/mozilla/RefPtr.h:50:40
    #3 0x7f76d1282501 in Release /builds/worker/workspace/obj-build/dist/include/mozilla/RefPtr.h:381:36
    #4 0x7f76d1282501 in assign_assuming_AddRef /builds/worker/workspace/obj-build/dist/include/mozilla/RefPtr.h:69:7
    #5 0x7f76d1282501 in operator= /builds/worker/workspace/obj-build/dist/include/mozilla/RefPtr.h:168:5
    #6 0x7f76d1282501 in mozilla::ClearOnShutdown_Internal::PointerClearer<RefPtr<mozilla::extensions::WebRequestService> >::Shutdown() /builds/worker/workspace/obj-build/dist/include/mozilla/ClearOnShutdown.h:72:13
    #7 0x7f76c5846dad in mozilla::KillClearOnShutdown(mozilla::ShutdownPhase) /builds/worker/checkouts/gecko/xpcom/base/ClearOnShutdown.cpp:53:19
    #8 0x7f76c5ab3b6d in mozilla::ShutdownXPCOM(nsIServiceManager*) /builds/worker/checkouts/gecko/xpcom/build/XPCOMInit.cpp:662:5
    #9 0x7f76d176978d in ~ScopedXPCOM /builds/worker/checkouts/gecko/tools/fuzzing/interface/harness/FuzzerTestHarness.h:106:21
    #10 0x7f76d176978d in mozilla::_InitFuzzer::DeinitXPCOM() /builds/worker/checkouts/gecko/tools/fuzzing/interface/harness/FuzzerRunner.cpp:25:23
    #11 0x7f76e2351db6 in __run_exit_handlers (/usr/lib/libc.so.6+0x3fdb6)
    #12 0x7f76e2351f5d in exit (/usr/lib/libc.so.6+0x3ff5d)
    #13 0x55d833a9f4a5 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /builds/worker/checkouts/gecko/tools/fuzzing/libfuzzer/FuzzerDriver.cpp
    #14 0x7f76d176904f in mozilla::FuzzerRunner::Run(int*, char***) /builds/worker/checkouts/gecko/tools/fuzzing/interface/harness/FuzzerRunner.cpp:75:13
    #15 0x7f76d16aa57e in XREMain::XRE_mainStartup(bool*) /builds/worker/checkouts/gecko/toolkit/xre/nsAppRunner.cpp:3851:35
    #16 0x7f76d16bb268 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /builds/worker/checkouts/gecko/toolkit/xre/nsAppRunner.cpp:4941:12
    #17 0x7f76d16bc0b3 in XRE_main(int, char**, mozilla::BootstrapConfig const&) /builds/worker/checkouts/gecko/toolkit/xre/nsAppRunner.cpp:5008:21
    #18 0x55d833921f97 in do_main /builds/worker/checkouts/gecko/browser/app/nsBrowserApp.cpp:217:22
    #19 0x55d833921f97 in main /builds/worker/checkouts/gecko/browser/app/nsBrowserApp.cpp:331:16
    #20 0x7f76e233a151 in __libc_start_main (/usr/lib/libc.so.6+0x28151)
    #21 0x55d833876309 in _start (/home/tunnelshade/workspace/firefox/m-r-20201005120340-fuzzing-asan-opt/firefox+0xa4309)

0x604000092a50 is located 0 bytes inside of 48-byte region [0x604000092a50,0x604000092a80)
freed by thread T0 (MainThread) here:
    #0 0x55d8338eea3d in free /builds/worker/fetches/llvm-project/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:123:3
    #1 0x7f76e2351db6 in __run_exit_handlers (/usr/lib/libc.so.6+0x3fdb6)

previously allocated by thread T0 (MainThread) here:
    #0 0x55d8338eecbd in malloc /builds/worker/fetches/llvm-project/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:145:3
    #1 0x55d833924dad in moz_xmalloc /builds/worker/checkouts/gecko/memory/mozalloc/mozalloc.cpp:52:15
    #2 0x7f76d125d20c in operator new /builds/worker/workspace/obj-build/dist/include/mozilla/cxxalloc.h:33:10
    #3 0x7f76d125d20c in mozilla::extensions::WebRequestService::GetSingleton() /builds/worker/checkouts/gecko/toolkit/components/extensions/webrequest/WebRequestService.cpp:23:16
    #4 0x7f76d126c56c in mozilla::extensions::StreamFilterParent::Create(mozilla::dom::ContentParent*, unsigned long, nsTSubstring<char16_t> const&) /builds/worker/checkouts/gecko/toolkit/components/extensions/webrequest/StreamFilterParent.cpp:116:18
    #5 0x7f76cd3d2a35 in mozilla::dom::ContentParent::RecvInitStreamFilter(unsigned long const&, nsTString<char16_t> const&, std::function<void (mozilla::ipc::Endpoint<mozilla::extensions::PStreamFilterChild>&&)>&&) /builds/worker/checkouts/gecko/dom/ipc/ContentParent.cpp:3903:3
    #6 0x7f76c710cd54 in mozilla::dom::PContentParent::OnMessageReceived(IPC::Message const&) /builds/worker/workspace/obj-build/ipc/ipdl/PContentParent.cpp:7626:57
    #7 0x7f76c529dbc3 in void mozilla::ipc::FuzzProtocol<mozilla::dom::ContentParent>(mozilla::dom::ContentParent*, unsigned char const*, unsigned long, nsTArray<nsTString<char> > const&) /builds/worker/workspace/obj-build/dist/include/ProtocolFuzzer.h:99:18
    #8 0x7f76c529d038 in RunContentParentIPCFuzzing(unsigned char const*, unsigned long) /builds/worker/checkouts/gecko/dom/ipc/fuzztest/content_parent_ipc_libfuzz.cpp:27:3
    #9 0x55d833aafa2e in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) /builds/worker/checkouts/gecko/tools/fuzzing/libfuzzer/FuzzerLoop.cpp:562:11
    #10 0x55d833a99d0e in fuzzer::RunOneTest(fuzzer::Fuzzer*, char const*, unsigned long) /builds/worker/checkouts/gecko/tools/fuzzing/libfuzzer/FuzzerDriver.cpp:301:6
    #11 0x55d833a9f441 in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) /builds/worker/checkouts/gecko/tools/fuzzing/libfuzzer/FuzzerDriver.cpp:802:9
    #12 0x7f76d176904f in mozilla::FuzzerRunner::Run(int*, char***) /builds/worker/checkouts/gecko/tools/fuzzing/interface/harness/FuzzerRunner.cpp:75:13
    #13 0x7f76d16aa57e in XREMain::XRE_mainStartup(bool*) /builds/worker/checkouts/gecko/toolkit/xre/nsAppRunner.cpp:3851:35
    #14 0x7f76d16bb268 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) /builds/worker/checkouts/gecko/toolkit/xre/nsAppRunner.cpp:4941:12
    #15 0x7f76d16bc0b3 in XRE_main(int, char**, mozilla::BootstrapConfig const&) /builds/worker/checkouts/gecko/toolkit/xre/nsAppRunner.cpp:5008:21
    #16 0x55d833921f97 in do_main /builds/worker/checkouts/gecko/browser/app/nsBrowserApp.cpp:217:22
    #17 0x55d833921f97 in main /builds/worker/checkouts/gecko/browser/app/nsBrowserApp.cpp:331:16
    #18 0x7f76e233a151 in __libc_start_main (/usr/lib/libc.so.6+0x28151)

SUMMARY: AddressSanitizer: heap-use-after-free /builds/worker/workspace/obj-build/dist/include/nsISupportsImpl.h:321:34 in operator--
Shadow bytes around the buggy address:
  0x0c088000a4f0: fa fa 00 00 00 00 00 00 fa fa 00 00 00 00 00 00
  0x0c088000a500: fa fa 00 00 00 00 00 00 fa fa 00 00 00 00 00 00
  0x0c088000a510: fa fa 00 00 00 00 00 00 fa fa 00 00 00 00 00 06
  0x0c088000a520: fa fa 00 00 00 00 00 06 fa fa 00 00 00 00 00 06
  0x0c088000a530: fa fa 00 00 00 00 00 fa fa fa fd fd fd fd fd fd
=>0x0c088000a540: fa fa fd fd fd fd fd fa fa fa[fd]fd fd fd fd fd
  0x0c088000a550: fa fa 00 00 00 00 00 fa fa fa fd fd fd fd fd fa
  0x0c088000a560: fa fa fd fd fd fd fd fa fa fa fd fd fd fd fd fa
  0x0c088000a570: fa fa fd fd fd fd fd fa fa fa 00 00 00 00 00 00
  0x0c088000a580: fa fa fd fd fd fd fd fa fa fa 00 00 00 00 00 fa
  0x0c088000a590: fa fa 00 00 00 00 00 fa fa fa fd fd fd fd fd fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==560090==ABORTING

Speculative Root Cause

Ref counting on a stale reference to WebRequestService object during xpcom shutdown. I am not sure if this can be reproduced in a running browser instance and not just the fuzzing harness.

Flags: sec-bounty?
Type: task → defect
Component: Security → Request Handling
Product: Firefox → WebExtensions

maybe sec-moderate in the worst case? I think the extension process would only shut down at browser shut down which doesn't seem very useful even if this does happen to be exploitable.

Flags: needinfo?(mixedpuppy)
Keywords: csectype-uaf, sec-moderate

I am unable to reproduce the issue with the attached test case and the latest m-c build using the stock ContentParentIPC harness.

I retested against latest central. I have included a poc for this specific build.

[2020-10-09 12:00:40] > Task ID: YeeNhNmkR5Wn3mxOe0R79A
[2020-10-09 12:00:40] > Rank: 1602217074
[2020-10-09 12:00:40] > Changeset: 600f47bbfeb2b8dd8feb52dc9b0df0c72e01da9e
[2020-10-09 12:00:40] > Build ID: 20201009041754

Attachment: 600f47bbfeb2b8dd8feb52dc9b0df0c72e01da9e-m-c

For every different build, messageId changes. So need to change that in poc too.

Thank you, I have successfully reproduced the issue.

A Pernosco session is available here: https://pernos.co/debug/kbopJ9xgR8m1MMFBUCk-6A/index.html

Status: UNCONFIRMED → NEW
Ever confirmed: true

It looks that the WebRequestService RefPtr is initially created by a call to ContentParent::RecvInitStreamFilter released, after that point no additional references are added to that RefPtr because we are just returning a reference to the *sWeakWebRequestService.

When the FuzzerDriver.cpp is exiting here: https://searchfox.org/mozilla-central/rev/c2e3ac11be4837718c2604e58085fbc8252b69dd/tools/fuzzing/libfuzzer/FuzzerDriver.cpp#812, the last reference to the WebRequestService RefPtr is released and right after that the WebRequestService destructor is executed.

The use after free seems to be triggered while executing NS_ShutdownXPCOM, which is due to ClearOnShutdown:

I have the feeling that this may not be something that would happen outside of the fuzzing harness, I would expect NS_ShutdownXPCOM to be called before exiting the process in a regular browser instance not running under the fuzzing harness.

Hi Christian Holler (or Tyson or tunnelshade),
does the rationale in comment 6 make sense from your perspective?

we'll wait your confirmation before setting a severity and priority on this issue.

Besides that, is it possible to reproduce this issue locally? (e.g. is there a doc page to provide guidance on reproducing fuzzing tests locally?)

Flags: needinfo?(twsmith)
Flags: needinfo?(tunnelshade)
Flags: needinfo?(choller)

clearing needinfo assigned to Shane, while we are looking into it.

Flags: needinfo?(mixedpuppy)

(In reply to Luca Greco [:rpl] [:luca] [:lgreco] from comment #6)

I have the feeling that this may not be something that would happen outside of the fuzzing harness, I would expect NS_ShutdownXPCOM to be called before exiting the process in a regular browser instance not running under the fuzzing harness.

Yes, this is very much possible and we had similar problems with the ContentParentIPC and other IPC fuzzing targets before.

We do register an at-exit handler because of that exit call in libFuzzer here:

https://searchfox.org/mozilla-central/rev/819be4899a92213abf121b449779ced662f2ce13/tools/fuzzing/interface/harness/FuzzerRunner.cpp#46

Do you think that DeinitXPCOM method needs a call to NS_ShutdownXPCOM ?

Flags: needinfo?(twsmith)
Flags: needinfo?(tunnelshade)
Flags: needinfo?(lgreco)
Flags: needinfo?(choller)

(In reply to Christian Holler (:decoder) from comment #9)

(In reply to Luca Greco [:rpl] [:luca] [:lgreco] from comment #6)

I have the feeling that this may not be something that would happen outside of the fuzzing harness, I would expect NS_ShutdownXPCOM to be called before exiting the process in a regular browser instance not running under the fuzzing harness.

Yes, this is very much possible and we had similar problems with the ContentParentIPC and other IPC fuzzing targets before.

We do register an at-exit handler because of that exit call in libFuzzer here:

https://searchfox.org/mozilla-central/rev/819be4899a92213abf121b449779ced662f2ce13/tools/fuzzing/interface/harness/FuzzerRunner.cpp#46

Do you think that DeinitXPCOM method needs a call to NS_ShutdownXPCOM ?

Unfortunately I'm not really that much experienced on how xpcom shutdown sequence, and so I should definitely like to double-check it with something that has more direct knowledge about those pieces, but in the meantime I took a look to get an initial picture and if I'm not reading it wrong I think that DeinitXPCOM should be already calling NS_ShutdownXPCOM.

Would you mind to double-check if any of the following details are not right?

I did want to also double-check if the pernosco trace linked in comment 5 does confirm that the sequence described above is actually consistent with what is really happening during the fuzzy test shutdown, but I couldn't load successfully the Pernosco trace (I already asked in the Pernosco support channel about the issue I'm having on loading this trace, but I'll try to take another look if I can get access to the trace again).

Flags: needinfo?(lgreco)

(In reply to Christian Holler (:decoder) from comment #9)

Do you think that DeinitXPCOM method needs a call to NS_ShutdownXPCOM ?

The pernosco trace linked in comment 10 has been restored and so I did take another look:

  • the sequence described in comment 10 looks about right: FuzzerRunner is definitely calling NS_ShutdownXPCOM (and it does call ClearOnShutdown as expected, and this is actually where we are triggering the use after free when the fuzz test is exiting).

  • the only RefPtr<WebRequestService> reference has got added by the following call stack:

fuzzer::FuzzerDriver () at FuzzerDriver.cpp:802
fuzzer::RunOneTest () at FuzzerDriver.cpp:301
  ::Fuzzer::ExecuteCallback () at FuzzerLoop.cpp:562
RunContentParentIPCFuzzing () at content_parent_ipc_libfuzz.cpp:27
mozilla::ipc::FuzzProtocol<> () at ProtocolFuzzer.h:99
  ::PContentParent::OnMessageReceived () at PContentParent.cpp:7978
  ::ContentParent::RecvInitStreamFilter () at ContentParent.cpp:4041
  ::StreamFilterParent::Create () at StreamFilterParent.cpp:116
  ::WebRequestService::GetSingleton () at WebRequestService.cpp:23
  • FuzzerDriver call does exit before FuzzerRunner and so the NS_ShutdownXPCOM call from the FuzzerRunner's exit handler is being called after the FuzzerDriver exit handlers did already release the last (and only) reference to the RefPtr<WebRequestService> reference

I did also take a look to the reasons behind that static WebRequestService* sWeakWebRequestService:

It seems also something to take into account if we do plan to make changes to WebRequestService (in particular to make sure we don't reintroduce the issue that was tracked by Bug 1396652).

Flags: needinfo?(choller)

Looking into other places in mozilla-central where we are using ClearOnShutdown, as well as looking to the inline comment right above ClearOnShutdown definition here:

Although ClearOnShutdown will work with any smart pointer (i.e., nsCOMPtr, RefPtr, StaticRefPtr, and StaticAutoPtr),
you probably want to use it only with StaticRefPtr and StaticAutoPtr.
There is no way to undo a call to ClearOnShutdown, so you can call it only on smart pointers which you know will live until the program shuts down.
In practice, these are likely global variables, which should be Static{Ref,Auto}Ptr.

I'm wondering if static RefPtr<WebRequestService> instance should actually be changed to static StaticRefPtr<WebRequestService> instance (e..g. similarly to what we are doing in dom/indexedDB/ActorsParentCommon.cpp here), but that is something to double-check (if it would be technically correct and if it would make any actual difference for the issue triggered by the fuzzer test harness) with someone that does have more direct knowledge than me about CleanOnShutdown and StaticRefPtr, before make any actual plan to make any change to that.

Looking to the mercurial logs maybe :glandium would be able to help on double-checking that.

Mike, what are your thoughts on the proposed change in comment 12?

Flags: needinfo?(mh+mozilla)

100% static RefPtr<> is not good. We had a discussion recently with Nika about these sort of things. It would seem like we really should do some static analysis on this pattern, because that's not the only place where this is happening.

That said, with a combination of static StaticRefPtr and ClearOnShutdown, I would still expect things to be able to go wrong. In fact, as far as destructors are concerned, static RefPtr shouldn't be different from static StaticRefPtr given we don't unload libxul anymore.

Flags: needinfo?(mh+mozilla)
Assignee: nobody → lgreco
Status: NEW → ASSIGNED

Clearing old needinfo: in the meantime I have attached a patch with a proposed change to fix the issue on the WebRequestService side (and verified locally that with those changes applied the fuzzy asan crash isn't triggered anymore, some additional detail in this comment on phabricator: https://phabricator.services.mozilla.com/D94692#3053969).

Flags: needinfo?(choller)
Severity: -- → S2
Priority: -- → P2

https://hg.mozilla.org/integration/autoland/rev/8de8cd3371e801d408650f102df04252c846f33d
https://hg.mozilla.org/mozilla-central/rev/8de8cd3371e8

Group: firefox-core-security → core-security-release
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
status-firefox84: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → 84 Branch

This grafts cleanly to Beta & ESR78. Is this something we should consider for uplift?

status-firefox82: --- → wontfix
status-firefox83: --- → affected
status-firefox-esr78: --- → affected
Flags: needinfo?(lgreco)

(In reply to Ryan VanderMeulen [:RyanVM] from comment #18)

This grafts cleanly to Beta & ESR78. Is this something we should consider for uplift?

Honestly I'm not sure if this can be really (and easily) triggered in a real browser instance and be easily exploitable (I also tried to reproduce it with a small gtest and I wasn't able to trigger it).

I fixed it sooner rather than later mainly because I think that this issue may prevent the fuzzy tests to be able to catch other potential issues (because if this one is triggered the run crash and may miss other ones), and for that I guess that having it on Nightly may be enough.

Nevertheless, (after it did bake a bit on Nightly) I would not be against to uplift it if we want to, the risk of regressions should be low.

Let's see what is :dveditz opinion about uplifting it.

Flags: needinfo?(lgreco) → needinfo?(dveditz)
Flags: sec-bounty?
Flags: sec-bounty+
Flags: needinfo?(dveditz)

The patch landed in nightly and beta is affected.
:rpl, is this bug important enough to require an uplift?
If not please set status_beta to wontfix.

For more information, please visit auto_nag documentation.

Flags: needinfo?(lgreco)

Re-assigning the needinfo request (the question is in comment 19) as agreed over slack with Daniel.

Flags: needinfo?(lgreco) → needinfo?(dveditz)

Given the simple fix, that equally clearly announces which object gets prematurely freed, it would be safer to uplift this than not.

tracking-firefox84: --- → +
Flags: needinfo?(dveditz)

(In reply to Daniel Veditz [:dveditz] from comment #22)

Given the simple fix, that equally clearly announces which object gets prematurely freed, it would be safer to uplift this than not.

Thanks Daniel, I see your point and uplifting the fix does sound good to me too.

Comment on attachment 9183660 [details]
Bug 1669466 - Change WebRequestService singleton to a StaticRefPtr. r=glandium!,mixedpuppy!

Beta/Release Uplift Approval Request

  • User impact if declined: The issue to be leveraged for an exploit (if that turns out to be doable).
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce: I've not been able to find an easy way to reproduce the issue outside of the fuzzy tests. I did verify the fix by applying the changes to a mozilla-central clone checked out on the same version used to run the fuzzy test and collect the data attached to the bug.
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): - the actual change is small and trivial
  • all the existing tests does pass successfully
  • verified fuzzy test failure is fixed on a local build
  • the fix did bake on nightly a bit (and we are not aware of any new issue related to this been reported on bugzilla so far)
  • String changes made/needed:
Attachment #9183660 - Flags: approval-mozilla-beta?

Comment on attachment 9183660 [details]
Bug 1669466 - Change WebRequestService singleton to a StaticRefPtr. r=glandium!,mixedpuppy!

ESR Uplift Approval Request

  • If this is not a sec:{high,crit} bug, please state case for ESR consideration: Security expressed in favor of uplifting it (See https://bugzilla.mozilla.org/show_bug.cgi?id=1669466#c22)
  • User impact if declined: The issue to be leveraged for an exploit (if that turns out to be doable).
  • Fix Landed on Version: 84
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): risk set to low for the following reasons:
  • the actual change is small and trivial (and the code changed and the directly surrounding code didn't change much from the version in Firefox 78, the context and rationale for the change as described in the phabricator comments do also apply unchanged to Firefox 78)
  • all the existing tests does pass successfully
  • verified fuzzy test failure is fixed on a local build
  • the fix did bake on nightly a bit (and we are not aware of any new issue related to this been reported on bugzilla so far)
  • String or UUID changes made by this patch:
Attachment #9183660 - Flags: approval-mozilla-esr78?

Comment on attachment 9183660 [details]
Bug 1669466 - Change WebRequestService singleton to a StaticRefPtr. r=glandium!,mixedpuppy!

approved for 83.0b9 and 78.5esr

Attachment #9183660 - Flags: approval-mozilla-esr78?
Attachment #9183660 - Flags: approval-mozilla-esr78+
Attachment #9183660 - Flags: approval-mozilla-beta?
Attachment #9183660 - Flags: approval-mozilla-beta+
Whiteboard: [reporter-external] [client-bounty-form] [verif?] → [reporter-external] [client-bounty-form] [verif?][adv-main83+]
Attached file advisory.txt —
Whiteboard: [reporter-external] [client-bounty-form] [verif?][adv-main83+] → [reporter-external] [client-bounty-form] [verif?][adv-main83+][adv-esr78.5+]
Alias: CVE-2020-26959
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: