heap-use-after-free in [@ mozilla::a11y::DocAccessible::DoARIAOwnsRelocation]

RESOLVED FIXED in Firefox -esr52

Status

()

defect
P1
critical
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: tsmith, Assigned: eeejay)

Tracking

(Blocks 1 bug, 5 keywords)

Trunk
mozilla57
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite ?
qe-verify -

Firefox Tracking Flags

(firefox-esr5256+ fixed, firefox55 wontfix, firefox56+ fixed, firefox57+ fixed)

Details

(Whiteboard: [adv-main56+][adv-esr52.4+][post-critsmash-triage])

Attachments

(4 attachments, 2 obsolete attachments)

Posted file test_case.html
The attached testcase can take up to 30 seconds to repro the issue.

==15360==ERROR: AddressSanitizer: heap-use-after-free on address 0x60200062c630 at pc 0x7f98411555d6 bp 0x7fffb44b2430 sp 0x7fffb44b2428
READ of size 8 at 0x60200062c630 thread T0
    #0 0x7f98411555d5 in Length src/obj-firefox/dist/include/nsTArray.h:398:37
    #1 0x7f98411555d5 in mozilla::a11y::DocAccessible::DoARIAOwnsRelocation(mozilla::a11y::Accessible*) src/accessible/generic/DocAccessible.cpp:2081
    #2 0x7f98410c2f2c in mozilla::a11y::NotificationController::WillRefresh(mozilla::TimeStamp) src/accessible/base/NotificationController.cpp:811:18
    #3 0x7f983def2566 in nsRefreshDriver::Tick(long, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:1856:12
    #4 0x7f983df013c5 in mozilla::RefreshDriverTimer::TickRefreshDrivers(long, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) src/layout/base/nsRefreshDriver.cpp:300:7
    #5 0x7f983df01082 in mozilla::RefreshDriverTimer::Tick(long, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:321:5
    #6 0x7f983df0371b in RunRefreshDrivers src/layout/base/nsRefreshDriver.cpp:763:5
    #7 0x7f983df0371b in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:676
    #8 0x7f983defebd7 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::ParentProcessVsyncNotifier::Run() src/layout/base/nsRefreshDriver.cpp:522:20
    #9 0x7f98373321be in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1446:14
    #10 0x7f9837338358 in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:480:10
    #11 0x7f983813e231 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:97:21
    #12 0x7f98380a012b in RunInternal src/ipc/chromium/src/base/message_loop.cc:326:10
    #13 0x7f98380a012b in RunHandler src/ipc/chromium/src/base/message_loop.cc:319
    #14 0x7f98380a012b in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:299
    #15 0x7f983d8027ff in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:156:27
    #16 0x7f98418e9cd1 in nsAppStartup::Run() src/toolkit/components/startup/nsAppStartup.cpp:287:30
    #17 0x7f9841acba14 in XREMain::XRE_mainRun() src/toolkit/xre/nsAppRunner.cpp:4608:22
    #18 0x7f9841acd618 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:4772:8
    #19 0x7f9841acea4b in XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:4867:21
    #20 0x4eb643 in do_main src/browser/app/nsBrowserApp.cpp:236:22
    #21 0x4eb643 in main src/browser/app/nsBrowserApp.cpp:309
    #22 0x7f98547d382f in __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c:291
    #23 0x41d198 in _start (/home/user/workspace/browsers/m-c-1502010358-asan-opt/firefox+0x41d198)

0x60200062c630 is located 0 bytes inside of 8-byte region [0x60200062c630,0x60200062c638)
freed by thread T0 here:
    #0 0x4bb6cb in __interceptor_free /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:47:3
    #1 0x7f983721c3df in RawRemove src/xpcom/ds/PLDHashTable.cpp:662:3
    #2 0x7f983721c3df in PLDHashTable::RemoveEntry(PLDHashEntryHdr*) src/xpcom/ds/PLDHashTable.cpp:644
    #3 0x7f9841155dce in RemoveEntry src/obj-firefox/dist/include/nsTHashtable.h:222:12
    #4 0x7f9841155dce in Remove src/obj-firefox/dist/include/nsBaseHashtable.h:171
    #5 0x7f9841155dce in mozilla::a11y::DocAccessible::MoveChild(mozilla::a11y::Accessible*, mozilla::a11y::Accessible*, int) src/accessible/generic/DocAccessible.cpp:2274
    #6 0x7f984115527f in mozilla::a11y::DocAccessible::DoARIAOwnsRelocation(mozilla::a11y::Accessible*) src/accessible/generic/DocAccessible.cpp:2162:9
    #7 0x7f98410c2f2c in mozilla::a11y::NotificationController::WillRefresh(mozilla::TimeStamp) src/accessible/base/NotificationController.cpp:811:18
    #8 0x7f983def2566 in nsRefreshDriver::Tick(long, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:1856:12
    #9 0x7f983df013c5 in mozilla::RefreshDriverTimer::TickRefreshDrivers(long, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) src/layout/base/nsRefreshDriver.cpp:300:7
    #10 0x7f983df01082 in mozilla::RefreshDriverTimer::Tick(long, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:321:5
    #11 0x7f983df0371b in RunRefreshDrivers src/layout/base/nsRefreshDriver.cpp:763:5
    #12 0x7f983df0371b in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:676
    #13 0x7f983defebd7 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::ParentProcessVsyncNotifier::Run() src/layout/base/nsRefreshDriver.cpp:522:20
    #14 0x7f98373321be in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1446:14
    #15 0x7f9837338358 in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:480:10
    #16 0x7f983813e231 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:97:21
    #17 0x7f98380a012b in RunInternal src/ipc/chromium/src/base/message_loop.cc:326:10
    #18 0x7f98380a012b in RunHandler src/ipc/chromium/src/base/message_loop.cc:319
    #19 0x7f98380a012b in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:299
    #20 0x7f983d8027ff in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:156:27
    #21 0x7f98418e9cd1 in nsAppStartup::Run() src/toolkit/components/startup/nsAppStartup.cpp:287:30
    #22 0x7f9841acba14 in XREMain::XRE_mainRun() src/toolkit/xre/nsAppRunner.cpp:4608:22
    #23 0x7f9841acd618 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:4772:8
    #24 0x7f9841acea4b in XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:4867:21
    #25 0x4eb643 in do_main src/browser/app/nsBrowserApp.cpp:236:22
    #26 0x4eb643 in main src/browser/app/nsBrowserApp.cpp:309
    #27 0x7f98547d382f in __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c:291

previously allocated by thread T0 here:
    #0 0x4bba1c in malloc /builds/slave/moz-toolchain/src/llvm/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:64:3
    #1 0x4ecf3d in moz_xmalloc src/memory/mozalloc/mozalloc.cpp:83:17
    #2 0x7f98411557d0 in operator new src/obj-firefox/dist/include/mozilla/mozalloc.h:200:12
    #3 0x7f98411557d0 in nsTArray<RefPtr<mozilla::a11y::Accessible> >* nsClassHashtable<nsPtrHashKey<mozilla::a11y::Accessible>, nsTArray<RefPtr<mozilla::a11y::Accessible> > >::LookupOrAdd<>(mozilla::a11y::Accessible*) src/obj-firefox/dist/include/nsClassHashtable.h:74
    #4 0x7f9841154b1c in mozilla::a11y::DocAccessible::DoARIAOwnsRelocation(mozilla::a11y::Accessible*) src/accessible/generic/DocAccessible.cpp:2075:56
    #5 0x7f98410c2f2c in mozilla::a11y::NotificationController::WillRefresh(mozilla::TimeStamp) src/accessible/base/NotificationController.cpp:811:18
    #6 0x7f983def2566 in nsRefreshDriver::Tick(long, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:1856:12
    #7 0x7f983df013c5 in mozilla::RefreshDriverTimer::TickRefreshDrivers(long, mozilla::TimeStamp, nsTArray<RefPtr<nsRefreshDriver> >&) src/layout/base/nsRefreshDriver.cpp:300:7
    #8 0x7f983df01082 in mozilla::RefreshDriverTimer::Tick(long, mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:321:5
    #9 0x7f983df0371b in RunRefreshDrivers src/layout/base/nsRefreshDriver.cpp:763:5
    #10 0x7f983df0371b in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::TickRefreshDriver(mozilla::TimeStamp) src/layout/base/nsRefreshDriver.cpp:676
    #11 0x7f983defebd7 in mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::ParentProcessVsyncNotifier::Run() src/layout/base/nsRefreshDriver.cpp:522:20
    #12 0x7f98373321be in nsThread::ProcessNextEvent(bool, bool*) src/xpcom/threads/nsThread.cpp:1446:14
    #13 0x7f9837338358 in NS_ProcessNextEvent(nsIThread*, bool) src/xpcom/threads/nsThreadUtils.cpp:480:10
    #14 0x7f983813e231 in mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) src/ipc/glue/MessagePump.cpp:97:21
    #15 0x7f98380a012b in RunInternal src/ipc/chromium/src/base/message_loop.cc:326:10
    #16 0x7f98380a012b in RunHandler src/ipc/chromium/src/base/message_loop.cc:319
    #17 0x7f98380a012b in MessageLoop::Run() src/ipc/chromium/src/base/message_loop.cc:299
    #18 0x7f983d8027ff in nsBaseAppShell::Run() src/widget/nsBaseAppShell.cpp:156:27
    #19 0x7f98418e9cd1 in nsAppStartup::Run() src/toolkit/components/startup/nsAppStartup.cpp:287:30
    #20 0x7f9841acba14 in XREMain::XRE_mainRun() src/toolkit/xre/nsAppRunner.cpp:4608:22
    #21 0x7f9841acd618 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:4772:8
    #22 0x7f9841acea4b in XRE_main(int, char**, mozilla::BootstrapConfig const&) src/toolkit/xre/nsAppRunner.cpp:4867:21
    #23 0x4eb643 in do_main src/browser/app/nsBrowserApp.cpp:236:22
    #24 0x4eb643 in main src/browser/app/nsBrowserApp.cpp:309
    #25 0x7f98547d382f in __libc_start_main /build/glibc-bfm8X4/glibc-2.23/csu/../csu/libc-start.c:291

SUMMARY: AddressSanitizer: heap-use-after-free src/obj-firefox/dist/include/nsTArray.h:398:37 in Length
Shadow bytes around the buggy address:
  0x0c04800bd870: fa fa fd fd fa fa 00 00 fa fa fd fd fa fa fd fa
  0x0c04800bd880: fa fa fd fd fa fa fd fa fa fa fd fa fa fa fd fd
  0x0c04800bd890: fa fa 00 00 fa fa 00 00 fa fa 00 00 fa fa fd fd
  0x0c04800bd8a0: fa fa fa fa fa fa fd fa fa fa fa fa fa fa fa fa
  0x0c04800bd8b0: fa fa fa fa fa fa fa fa fa fa 00 00 fa fa fa fa
=>0x0c04800bd8c0: fa fa fd fd fa fa[fd]fa fa fa 00 00 fa fa fd fd
  0x0c04800bd8d0: fa fa 00 00 fa fa fd fd fa fa 00 00 fa fa fd fa
  0x0c04800bd8e0: fa fa fd fa fa fa fd fd fa fa fd fd fa fa fd fd
  0x0c04800bd8f0: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fa
  0x0c04800bd900: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd
  0x0c04800bd910: fa fa fd fd fa fa fd fd fa fa fd fd fa fa fd fd
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
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  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
==15360==ABORTING
Flags: in-testsuite?
Assignee: nobody → eitan
OK, this patch adds a quick fix for the UAF, and also tries to fix the underlying case that allows us to get there.

We reach this state because of a race where sub#b is removed and then inserted with along with its children. Because sub#b is removed, both it, and its children are scheduled to be inserted into its parent, sub#a.

The accessible tree looks like this initially:
['sub@id="a" node', role: text]
  ['mark@id="e" node', role: text]
  ['sub@id="b" node', role: text]

sub#b is skipped in the process insertions stage because it is aria-owned, and is scheduled to be recreated later.

After all scheduled insertions go, it looks something like this:
['sub@id="a" node', role: text]
  [text leaf "X"]
  ['mark@id="e" node', role: text]
  [text leaf "\n"]
  [text leaf "Z"]
  [text leaf "\n"]

The exact order changes depending on how the insertions are scheduled. If mark#e is in index 0, |DoARIAOwnsRelocation| iterates over the aria-owns IDs, recreates mark#b, and reclaims all its children. So after the brief WTF state above, we get a correct tree.

If mark#e is *not* in index 0, we get to a condition where we replace the owned array. The while loop continues with a freed array, and kaboom.
Attachment #8895144 - Flags: review?(surkov.alexander)
I feel 80% certain that my insertion changes are good, but not 100%. As for the UAF fix, I feel 100% good about that.

I don't want to dwell much on the former, so if it is not the correct solution, I would prefer to fix the UAF in this bug and move on.
so, there are two parts in the patch, do they both independently fix the bug? I'm not sure I understand the first part, the second one is about that MoveChild may destroy ARIA owns array if I get it right. If so, then what we want to do about the assertion? Do we consider the case a legal one or we need to find a proper fix for it (the first patch's part I assume)?
Priority: -- → P1
Yeah, the second part remedies the UAF, which should never happen. Ever.

We reach the UAF because we get to a bad state. The relocated child is in the middle of non-relocated children(*). This is illegal!

In debug builds we would abort earlier because of failed assertions.

*) They are children of a relocated child that were flattened out in the last content insertion.
ok, so how do you want to approach to this bug? put a remedy patch (possibly backport it), and then work on a followup in antoher bug.

btw, for the remedy path: we don't need 'relocated' variable, we can keep 'owned' and the assertion and that's it.

Also we have another MoveChild case in PutChildrenBack, which may (will) make a kaboom one day as well.
(In reply to alexander :surkov from comment #5)
> ok, so how do you want to approach to this bug? put a remedy patch (possibly
> backport it), and then work on a followup in antoher bug.
> 
> btw, for the remedy path: we don't need 'relocated' variable, we can keep
> 'owned' and the assertion and that's it.

What would the assertion compare against if we don't store the new array in a temporary variable?
MOZ_ASSERT(owned == mARIAOwnsHash.Get(aOwner), "Blabla")
(In reply to alexander :surkov from comment #7)
> MOZ_ASSERT(owned == mARIAOwnsHash.Get(aOwner), "Blabla")

So do that, and then LookupOrAdd? Isn't that redundant? I guess the assert lookup would only be on debug builds.
(In reply to Eitan Isaacson [:eeejay] from comment #8)
> (In reply to alexander :surkov from comment #7)
> > MOZ_ASSERT(owned == mARIAOwnsHash.Get(aOwner), "Blabla")
> 
> So do that, and then LookupOrAdd? Isn't that redundant? I guess the assert
> lookup would only be on debug builds.

in what sense redundant? it's just an assertion, which is for debug builds only. The point is to keep the builds clean as possible. If you can do without extra local variables, then it's the way to go I'd say, and also it doesn't make sense to have variables in release builds if they are solely for debug builds.
did we decide to go with a small fix here and deal with the root problem in a follow up?
Flags: needinfo?(eitan)
(In reply to alexander :surkov from comment #10)
> did we decide to go with a small fix here and deal with the root problem in
> a follow up?

Yes. I'll update the patch. Sorry, was busy yesterday with other bugs.
Flags: needinfo?(eitan)
Focusing on the UAF here. After this lands I'll open a bug with the assertion errors that precede this UAF, if Tyson doesn't beat me to it :)
Attachment #8896337 - Flags: review?(surkov.alexander)
Attachment #8895144 - Attachment is obsolete: true
Attachment #8895144 - Flags: review?(surkov.alexander)
Comment on attachment 8896337 [details] [diff] [review]
Retrieve current array for owner in aria owns hash table. r?surkov

Review of attachment 8896337 [details] [diff] [review]:
-----------------------------------------------------------------

r=me, thanks! will you or want me to file follow ups?
Attachment #8896337 - Flags: review?(surkov.alexander) → review+
Posted patch Release patchSplinter Review
Attachment #8896351 - Flags: review?(surkov.alexander)
Posted patch Beta patch (obsolete) — Splinter Review
Attachment #8896353 - Flags: review?(surkov.alexander)
(In reply to alexander :surkov from comment #13)
> Comment on attachment 8896337 [details] [diff] [review]
> Retrieve current array for owner in aria owns hash table. r?surkov
> 
> Review of attachment 8896337 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> r=me, thanks! will you or want me to file follow ups?

Lets hold off on followups until after this lands. The test case here demonstrates a vulnerability. After it is fixed we can deal with the assertion errors.
The patch in this bug offers a more complete fix to the issue found in bug 1363723. I propose only backporting this patch, instead of the ones in bug 1363723.
See Also: → CVE-2017-7818
Comment on attachment 8896337 [details] [diff] [review]
Retrieve current array for owner in aria owns hash table. r?surkov

[Security approval request comment]
How easily could an exploit be constructed based on the patch?

Medium to hard? I don't think it is a blueprint of how to write an exploit in JS, but the UAF protection is apparent in the patch.

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

I don't think so.

Which older supported branches are affected by this flaw?

This was introduced in 48:
https://bugzilla.mozilla.org/show_bug.cgi?id=1256461

If not all supported branches, which bug introduced the flaw?

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

I have backports I can nominate after this lands.

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

I think this is a pretty safe patch.
Attachment #8896337 - Flags: sec-approval?
Posted patch esr52 patchSplinter Review
beta/release/esr52 are almost the same patch and should apply cleanly.
Attachment #8896351 - Flags: review?(surkov.alexander) → review+
Attachment #8896353 - Flags: review?(surkov.alexander) → review+
(In reply to Eitan Isaacson [:eeejay] from comment #16)

> > r=me, thanks! will you or want me to file follow ups?
> 
> Lets hold off on followups until after this lands. The test case here
> demonstrates a vulnerability. After it is fixed we can deal with the
> assertion errors.

they also probably should be/could be filed as sec bugs
Sec-approval+ for checkin on August 28, three weeks into the current cycle (as a sec-high with a clear UAF fix in the patch). 
We'll want a ESR52 and Beta patches made and nominated as well.
Whiteboard: [checkin on 8/28]
Attachment #8896337 - Flags: sec-approval? → sec-approval+
Comment on attachment 8896351 [details] [diff] [review]
Release patch

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1256461
[User impact if declined]: A possible use after free condition
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Not yet. This is scheduled to all land at once.
[Needs manual test from QE? If yes, steps to reproduce]: Run test case file attached to this bug with accessibility enabled.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: It is a simple and small change.
[String changes made/needed]:
Attachment #8896351 - Flags: approval-mozilla-release?
Comment on attachment 8896353 [details] [diff] [review]
Beta patch

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1256461
[User impact if declined]: A possible use after free condition
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Not yet. This is scheduled to all land at once.
[Needs manual test from QE? If yes, steps to reproduce]: Run test case file attached to this bug with accessibility enabled.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: It is a simple and small change.
[String changes made/needed]:
Attachment #8896353 - Flags: approval-mozilla-beta?
Comment on attachment 8896359 [details] [diff] [review]
esr52 patch

[Approval Request Comment]
If this is not a sec:{high,crit} bug, please state case for ESR consideration:
User impact if declined: A possible use after free condition
Fix Landed on Version: Will be landed on all channels.
Risk to taking this patch (and alternatives if risky): This is a minimal change and it shouldn't be risky.
String or UUID changes made by this patch:
Attachment #8896359 - Flags: approval-mozilla-esr52?
Comment on attachment 8896351 [details] [diff] [review]
Release patch

Reading Al's comment, I don't think we want to take it to release.
Attachment #8896351 - Flags: approval-mozilla-release? → approval-mozilla-release-
Comment on attachment 8896353 [details] [diff] [review]
Beta patch

This patch will land after bug 1363723. So it will need to be rebased. The original patch will probably apply better.
Attachment #8896353 - Attachment is obsolete: true
Attachment #8896353 - Flags: approval-mozilla-beta?
Comment on attachment 8896337 [details] [diff] [review]
Retrieve current array for owner in aria owns hash table. r?surkov

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1256461
[User impact if declined]: A possible use after free condition
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: Not yet. This is scheduled to all land at once.
[Needs manual test from QE? If yes, steps to reproduce]: Run test case file attached to this bug with accessibility enabled.
[List of other uplifts needed for the feature/fix]: None.
[Is the change risky?]: No.
[Why is the change risky/not risky?]: It is a simple and small change.
[String changes made/needed]:
Attachment #8896337 - Flags: approval-mozilla-beta?
Comment on attachment 8896359 [details] [diff] [review]
esr52 patch

sec-high uaf fix for esr52.4
Attachment #8896359 - Flags: approval-mozilla-esr52? → approval-mozilla-esr52+
Comment on attachment 8896337 [details] [diff] [review]
Retrieve current array for owner in aria owns hash table. r?surkov

sec-high uaf fix for beta56
Attachment #8896337 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
https://hg.mozilla.org/mozilla-central/rev/fb5977ce5d1e
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
Group: dom-core-security → core-security-release
Whiteboard: [adv-main56+][adv-esr52.4+]
Flags: qe-verify-
Whiteboard: [adv-main56+][adv-esr52.4+] → [adv-main56+][adv-esr52.4+][post-critsmash-triage]
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.