Closed Bug 1482122 Opened 2 years ago Closed 2 years ago

Crash in nsTArray_Impl<T>::Clear | wcsrtombs | mozilla::jni::detail::ProxyNativeCall<T>::operator()


(Firefox for Android :: General, defect)

Not set



Firefox 64
Tracking Status
firefox61 --- wontfix
firefox62 --- wontfix
firefox63 --- verified
firefox64 --- fixed


(Reporter: marcia, Assigned: gsvelto)



(Keywords: crash, csectype-uaf, sec-high, Whiteboard: [adv-main63+] fixed by bug 1476250)

Crash Data

This bug was filed from the Socorro interface and is
report bp-8f7aae82-8063-4131-9ebe-f0ae30180809.

Seen while looking at Android crash stats on release: Not sure where to bucket it, but it appears many of the signatures are potential UAFs so marking security sensitive. Only one crash seen in b62 so far.

Unfortunately comments are not useful.  

Top 10 frames of crashing thread:

0 nsTArray_Impl<BroadcastListener*, nsTArrayInfallibleAllocator>::Clear 
1 wcsrtombs 
2 mozilla::jni::detail::ProxyNativeCall<mozilla::GeckoScreenOrientation, mozilla::java::GeckoScreenOrientation, true, false, short, short>::operator widget/android/jni/Natives.h:418
3 mozilla::detail::RunnableFunction<mozilla::jni::detail::ProxyNativeCall<mozilla::GeckoScreenOrientation, mozilla::java::GeckoScreenOrientation, true, false, short, short> >::Run xpcom/threads/nsThreadUtils.h:552
4 nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1090
6 wcsrtombs 
7 mozilla::detail::VariantImplementation<unsigned char, 0, const int, const char*, void > > mfbt/Variant.h:219
8 mozilla::PerformanceCounter::GetDispatchCount android-ndk/sources/cxx-stl/llvm-libc++/include/atomic:885
9 wcsrtombs 

Group: core-security → firefox-core-security
Product: Core → Firefox for Android
Version: 61 Branch → Trunk
Who should look at this explosive new crash?
Flags: needinfo?(snorp)
It seems like this may be a variant of bug 1476250. What do you think, :gsvelto?
Flags: needinfo?(snorp) → needinfo?(gsvelto)
It could be, most of the stack seems garbled except for the part about calling OnOrientationChange(). The crashes look like UAFs alright since >80% have an e5e5e5e5 value as the crashing address and this would be consistent with bug 1476250 which is basically the same problem: the last screen orientation observer is unregistered, the observer array is deleted and then something tries to touch. There's something important here I hadn't noticed in the other bug: the crashing thread is _not_ the main thread and that's just wrong because the HAL shouldn't be accessed from other threads. I'll first have to check where the JNI code is calling OnOrientationChange() because if it's not from the main thread then that should be sorted out first.
Flags: needinfo?(gsvelto)
Scratch what I wrote before, the notification is sent on the correct thread. I have a hard time figuring out how the race can possibly happen. The patch that I'm writing for bug 1476250 should address this issue too though by never destroying the list of observers as long as the service is in use thus entirely preventing the race.
Updating assignee based on comment 4. Thanks!
Assignee: nobody → gsvelto
The fix for this issue is undergoing review in bug 1476250 where I had already started working on it before this bug was filed.
I'm about to land the patch for bug 1476250, it should fix this too.
There have been no instances of this crash for versions including the fix in bug 1476250, closing as fixed.
Closed: 2 years ago
Resolution: --- → FIXED
Group: firefox-core-security → core-security-release
Flags: needinfo?(sorina.florean)
Tested with the following devices on 63.0b13:
- Motorola Nexus 6(Android 7.1.1)
- Nexus 9(Android 6.0.1)
- Sony Xperia Z5 Premium(Android 6.0.1)
- Xiaomi Mi4i(Android 5.0.2)
We didn't see the crash while performing the steps from bug 1476250.
Flags: needinfo?(sorina.florean)
Whiteboard: [adv-main63+]

Crashes seem to have stopped in 61 or 62. I suspect we can open this.

Flags: needinfo?(dveditz)
Group: core-security-release
Depends on: 1476250
Flags: needinfo?(dveditz)
Whiteboard: [adv-main63+] → [adv-main63+] fixed by bug 1476250
You need to log in before you can comment on or make changes to this bug.