fix: use-after-free in react-native-screens removal listener (Sentry: APP-9Y9) by mbSmaga · Pull Request #98632 · Expensify/App

mbSmaga · 2026-08-14T06:42:06Z

Explanation of Change

Adds a patches/react-native-screens/ patch fixing an unsynchronized lazy init in NativeProxy::nativeAddMutationsListener, which is behind the Android SIGSEGV in facebook::react::MountingCoordinator::pullTransaction on thread mqt_v_js (Sentry APP-9Y9).

On a cold launch two threads reach nativeAddMutationsListener concurrently: ScreensModule.initialize() calls setupFabric() on the module thread while onHostResume() dispatches the same call to the main thread. Both can pass the if (!screenRemovalListener_) null check, and the racing shared_ptr assignments tear — libc++ moves the object pointer and the control block as two independent words, so the member can end up holding one thread's pointer beside the other thread's control block while the losing temporary frees the listener it points at.

The listener is registered as a MountingCoordinator mounting override delegate, and that list is append-only (react-native core has no unregister API). The torn slot reports use_count=1, expired=0 forever, so weak_ptr::lock() keeps succeeding and the next pullTransaction virtual-dispatches through a recycled vtable slot. A listener destroyed normally through its own control block is harmless — it reports expired=1 and core null-checks that path — which is what identifies this as the init race rather than a teardown bug.

The patch makes the listener a process-lifetime singleton (a function-local static, so initialization is thread-safe and the entry in core's non-removable delegate list can never dangle). It holds a mutex-guarded swappable callback capturing the JNI global reference by value instead of this, so it cannot dereference a finalized NativeProxy. setListener returns an ownership token and invalidateNative() clears the callback only while it still owns it, so a stale proxy's late teardown cannot disarm a newer proxy's install. A disarmed listener passes the transaction through untouched.

On the crash signature. The same shouldOverridePullTransaction() virtual dispatch faults in more than one shape, depending on what recycles the freed block: SEGV_ACCERR with the fault address inside [anon:scudo:primary] ("trying to execute non-executable memory") when the recycled chunk holds pointer-dense data, and SEGV_MAPERR when it does not — either with an unmapped ASCII-looking jump target, or with fault address 0x0 when the vtable slot itself reads back as zero and the jump never happens. All three have been observed for this bug (the last confirmed at instruction level on arm64 in the upstream thread, where lr shows the blr never ran). The 0x0 variant in particular reads like an unrelated core-renderer null dereference, which is worth knowing when triaging future reports against this issue.

This is a backport of an upstream fix that has already been reviewed and merged by Software Mansion: software-mansion/react-native-screens#4413, merged as b3badd012f83679b12f4e29f2e28eceaa4830efd. It is not in a react-native-screens release yet (the latest tag, 4.27.0, was published before the merge), so it ships here as a patch against our current 4.25.0, and can be dropped as soon as we bump to the first release containing that commit. The patched files are byte-identical to the merged upstream files, with upstream's cpp/legacy/RNSScreenRemovalListener.* mapping to cpp/RNSScreenRemovalListener.* in the published package.

Fixed Issues

$ #93842
PROPOSAL: #93842 (comment)
PROPOSAL: #93842 (comment)

Platform coverage and HybridApp

The patched code is Android-only native C++ (react-native-screens JNI + the Fabric mounting override delegate). It is not compiled on iOS, and mWeb/desktop run the web bundle, so those platforms cannot exercise this code path. I also do not have access to a macOS machine. The checklist boxes below are all ticked per the repo's CI requirement (the checklist verifies each item was considered); what was and was not actually exercised is stated here and annotated inline.

Verified on a standalone Android build: the patch applies cleanly (scripts/applyPatches.sh, zero warnings), NativeProxy.cpp and RNSScreenRemovalListener.cpp compile for x86_64 with no compiler diagnostics, librnscreens.so is packaged and confirmed loaded at runtime, and the navigation pass below runs clean with no crash. I have not built the HybridApp/Mobile-Expensify variant locally — happy to verify against the ad-hoc build on this PR, or to follow whatever the reviewer prefers.

Tests

The crash is a race that reproduces on roughly 1 in 1000 cold launches, so these steps verify that screen removal still behaves correctly with the patch applied rather than attempting to demonstrate the crash is gone. See "Evidence" below for what does support the fix.

Build and run the Android app from this branch on a device or emulator, and sign in.
Open Search from the Home header, then press BACK. Repeat several times, varying how long you wait before pressing BACK — sometimes immediately, sometimes after the screen settles.
Open a chat, open a thread or details panel from it, then back out of the whole stack.
Open Account, drill two or three levels deep, then back out.
Open Search and press BACK twice in quick succession (~150 ms apart), while the pop animation is still running. Repeat a few times.
Hop between the bottom tabs (Home, Inbox, Spend, Workspaces, Account).
Open a screen, background the app, then foreground it and press BACK.
Verify throughout that screens are removed correctly: no stale screen visible behind the current one, no duplicated headers, BACK always navigates, and the app does not crash.

Evidence supporting the fix itself, since the above cannot demonstrate a 1-in-1000 race:

The upstream PR was reviewed and merged by the react-native-screens maintainers.
The race is removed by construction, not made less likely: the lazy-init branch is replaced by a function-local static, whose thread-safe initialization is guaranteed by the C++ standard.
Measured on the unpatched code, both threads ran the init branch in 7 of 9 cold launches in one affected app and 3 of those crashed; with the init serialized, 0/20 and 0/20. In an App dev build, double-init occurred on 1/150 launches unguarded vs 0/1000 with the fix, while the racy entry window still opened at an unchanged rate (30.0% vs 30.4%) — i.e. the fix removes the race rather than changing startup timing so the threads stop colliding.
Independent third-party verification on physical arm64 hardware, reported on the upstream PR: 6 crashes in 128 unpatched launches, 0 in 88 patched.

If you want to exercise the race directly rather than rely on the above, the cold-relaunch loop and the instrumentation used to measure it are in this comment. Note that the useful signal there is the double-init rate, not the crash: the crash is ~1 in 1000 cold launches, while the double-init that causes it is roughly 40x more frequent, and with this patch the lazy-init branch no longer exists to double-enter.

Verify that no errors appear in the JS console

Offline tests

N/A. This is a native Android patch to the Fabric mounting path and does not change any network or offline behavior.

QA Steps

Android Native only — this crash is in native Android code (react-native-screens JNI / Fabric mounting delegate) triggered by a cold-launch race; iOS, mWeb and MacOS do not compile or execute this code path.

On Android Native, sign in to a test account.
Navigate into and back out of several screens: Search, a chat and its thread/details panel, and Account settings a few levels deep.
Press BACK twice in quick succession while a pop animation is still running, a few times.
Switch between the bottom tabs, and background/foreground the app while a screen is open.
Verify screens are removed correctly (no stale screen behind the current one, no duplicated headers, BACK always works) and the app does not crash.

Verify that no errors appear in the JS console

PR Author Checklist

Screenshots/Videos

Android: Native

nav-pass-clean.mp4

Android: mWeb Chrome

N/A — this patch only changes native Android code (react-native-screens JNI / Fabric mounting delegate), which mWeb does not compile or execute.

iOS: Native

N/A — this patch only changes native Android code, which is not compiled on iOS.

iOS: mWeb Safari

N/A — this patch only changes native Android code, which mWeb does not compile or execute.

MacOS: Chrome / Safari

N/A — this patch only changes native Android code, which the web bundle does not compile or execute.

react-native-screens created its screen-removal listener with an unsynchronized lazy init in NativeProxy::nativeAddMutationsListener. On a cold launch two threads reach that function concurrently, because ScreensModule.initialize() calls setupFabric() on the module thread while onHostResume() dispatches the same call to the main thread. Both can pass the null check, and the racing shared_ptr assignments tear: libc++ moves the object pointer and the control block as two independent words, so the member can end up holding one thread's pointer beside the other thread's control block while the losing temporary frees the listener it points at. The listener is registered as a MountingCoordinator mounting override delegate, and that list is append-only - react-native core has no unregister API. The torn slot reports use_count=1, expired=0 forever, so weak_ptr::lock() keeps succeeding and the next pullTransaction virtual dispatches through a recycled vtable slot. That is the Android SIGSEGV in MountingCoordinator::pullTransaction reported in Expensify#93842. A listener destroyed normally through its own control block is harmless, since it reports expired=1 and core null-checks that path. Make the listener a process-lifetime singleton - a function-local static, so initialization is thread-safe and the entry in core's non-removable delegate list can never dangle. It now holds a mutex-guarded swappable callback that captures the JNI global reference by value instead of this, so it cannot dereference a finalized NativeProxy. setListener returns an ownership token and invalidateNative() clears the callback only while it still owns it, so a stale proxy's late teardown cannot disarm the callback a newer proxy installed. A disarmed listener passes the transaction through untouched. Backport of software-mansion/react-native-screens#4413, merged upstream as b3badd012f83679b12f4e29f2e28eceaa4830efd. Not in a react-native-screens release yet, so it ships here as a patch-package patch.

github-actions · 2026-08-14T06:42:20Z

⚠️ This PR is possibly changing native code and/or updating libraries, it may cause problems with HybridApp. Please check if any patch updates are required in the HybridApp repo and run an AdHoc build to verify that HybridApp will not break. Ask Contributor Plus for help if you are not sure how to handle this. ⚠️

github-actions · 2026-08-14T06:42:57Z

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

The field was left as a placeholder because the PR number was not known when the patch was created. PATCHES.md requires it so that a future dependency bump can find the PR that introduced the patch and decide whether it is still needed.

mbSmaga · 2026-08-14T07:10:55Z

I have read the CLA Document and I hereby sign the CLA

mbSmaga · 2026-08-14T07:25:32Z

Re: the HybridApp warning above — this PR adds a single patches/react-native-screens/ patch plus its details.md entry, and changes no other code.

The patched code is Android-only native C++ (react-native-screens JNI + the Fabric mounting override delegate), so iOS is unaffected — those files aren't compiled there.

Verified on a standalone Android build: the patch applies cleanly via scripts/applyPatches.sh with zero warnings, NativeProxy.cpp and RNSScreenRemovalListener.cpp compile for x86_64 with no compiler diagnostics, and librnscreens.so is packaged and loaded at runtime. The recording in the PR description is from that build.

I haven't been able to verify the HybridApp/Mobile-Expensify variant: the Verify Android/iOS HybridApp builds jobs are gated on !github.event.pull_request.head.repo.fork, so they skip on this PR, and I don't have a HybridApp setup locally. If someone on the team can trigger an ad-hoc/HybridApp build against this branch I'm happy to help chase anything it turns up — otherwise let me know what you'd prefer here.

mbSmaga · 2026-08-14T07:25:42Z

recheck

melvin-bot · 2026-08-14T07:30:41Z

@linhvovan29546 Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button]

linhvovan29546 · 2026-08-17T10:40:48Z

CLA Assistant / CLA / CLA (pull_request_target)
CLA Assistant / CLA / CLA (pull_request_target)Failing after 16s

@mbSmaga This workflow is failing. Could you please merge main again to retrigger it?

…oval-listener-uaf

mbSmaga · 2026-08-17T11:16:48Z

@linhvovan29546 Merged main (e0af838). The CLA check re-ran on the new head and still fails, but the error changed.

Before:
Committers of Pull Request number 98632 have to sign the CLA 📝

Now (run):
Error: Could not update the JSON file: Repository rule violations found

linhvovan29546 · 2026-08-18T07:21:03Z

Testing...

linhvovan29546 · 2026-08-18T08:09:00Z

Reviewer Checklist

I have verified the author checklist is complete (all boxes are checked off).
I verified the correct issue is linked in the ### Fixed Issues section above
I verified testing steps are clear and they cover the changes made in this PR
- I verified the steps for local testing are in the Tests section
- I verified the steps for Staging and/or Production testing are in the QA steps section
- I verified the steps cover any possible failure scenarios (i.e. verify an input displays the correct error message if the entered data is not correct)
- I turned off my network connection and tested it while offline to ensure it matches the expected behavior (i.e. verify the default avatar icon is displayed if app is offline)
I checked that screenshots or videos are included for tests on all platforms
I included screenshots or videos for tests on all platforms
I verified that the composer does not automatically focus or open the keyboard on mobile unless explicitly intended. This includes checking that returning the app from the background does not unexpectedly open the keyboard.
I verified tests pass on all platforms & I tested again on:
- Android: HybridApp
- Android: mWeb Chrome
- iOS: HybridApp
- iOS: mWeb Safari
- MacOS: Chrome / Safari
If there are any errors in the console that are unrelated to this PR, I either fixed them (preferred) or linked to where I reported them in Slack
I verified proper code patterns were followed (see Reviewing the code)
- I verified that comments were added to code that is not self explanatory
- I verified that any new or modified comments were clear, correct English, and explained "why" the code was doing something instead of only explaining "what" the code was doing.
- I verified any copy / text that was added to the app is grammatically correct in English. It adheres to proper capitalization guidelines (note: only the first word of header/labels should be capitalized), and is either coming verbatim from figma or has been approved by marketing (in order to get marketing approval, ask the Bug Zero team member to add the Waiting for copy label to the issue)
If a new code pattern is added I verified it was agreed to be used by multiple Expensify engineers
I verified that this PR follows the guidelines as stated in the Review Guidelines
I verified other components that can be impacted by these changes have been tested, and I retested again (i.e. if the PR modifies a shared library or component like Avatar, I verified the components using Avatar have been tested & I retested again)
If a new component is created I verified that:
- A similar component doesn't exist in the codebase
- All props are defined accurately
- The component has a clear name that is non-ambiguous and the purpose of the component can be inferred from the name alone
- The only data being stored in the state is data necessary for rendering and nothing else
- The component has the minimum amount of code necessary for its purpose, and it is broken down into smaller components in order to separate concerns and functions
If a new CSS style is added I verified that:
- A similar style doesn't already exist
- The style can't be created with an existing StyleUtils function (i.e. StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
If the PR modifies code that runs when editing or sending messages, I tested and verified there is no unexpected behavior for all supported markdown - URLs, single line code, code blocks, quotes, headings, bold, strikethrough, and italic.
If the PR modifies a generic component, I tested and verified that those changes do not break usages of that component in the rest of the App (i.e. if a shared library or component like Avatar is modified, I verified that Avatar is working as expected in all cases)
If the PR modifies a component related to any of the existing Storybook stories, I tested and verified all stories for that component are still working as expected.
If the PR modifies a component or page that can be accessed by a direct deeplink, I verified that the code functions as expected when the deeplink is used - from a logged in and logged out account.
If the PR modifies the UI (e.g. new buttons, new UI components, changing the padding/spacing/sizing, moving components, etc) or modifies the form input styles:
- I verified that all the inputs inside a form are aligned with each other.
- I added Design label and/or tagged @Expensify/design so the design team can review the changes.
For any bug fix or new feature in this PR, I verified that sufficient unit tests are included to prevent regressions in this flow.
If the main branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to the Test steps.
I have checked off every checkbox in the PR reviewer checklist, including those that don't apply to this PR.

Screenshots/Videos

Android: HybridApp

telegram-cloud-document-5-6061894507744471277.1.mp4

Android: mWeb Chrome

N/A the patch apply for android native only

iOS: HybridApp

N/A. The patch only applies to Android native code.

iOS: mWeb Safari

N/A. The patch only applies to Android native code.

MacOS: Chrome / Safari

N/A. The patch only applies to Android native code.

linhvovan29546 · 2026-08-18T08:57:46Z

CLA Assistant / CLA / CLA (pull_request_target)Failing after 18s

NAB: The CLA check failed because the required workflow Verify peer review, Verify peer review was not satisfied.

linhvovan29546

@mountiny Could you please trigger the Android ad hoc build? I’d like to test it on the ad hoc build as well.

mountiny · 2026-08-18T17:35:56Z

ran it the cla should work now

github-actions · 2026-08-18T17:36:32Z

🚧 mountiny has triggered a test Expensify/App build. You can view the workflow run here.

mountiny

Thanks for adding this!

github-actions · 2026-08-19T18:34:10Z

🚧 mountiny has triggered a test Expensify/App build. You can view the workflow run here.

OSBotify · 2026-08-19T18:37:16Z

✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release.

github-actions · 2026-08-19T18:48:06Z

🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
Built from App PR #98632.

Android 🤖	iOS 🍎
https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-android-Adhoc-2b031be-532a293-735d4470da71e5b959c806b9e3868cd4175e0649/index.html	https://ad-hoc-expensify-cash.s3.us-east-1.amazonaws.com/rock-artifacts/ad-hoc/rock-ios-device-AdHoc-2b031be-532a293-85211150e3cc0ce6682e3b2339652d24fffb3aad/index.html

Web 🕸️
https://98632.pr-testing.expensify.com

👀 View the workflow run that generated this build 👀

melvin-bot Bot assigned mbSmaga Aug 14, 2026

mbSmaga marked this pull request as ready for review August 14, 2026 07:30

mbSmaga requested a review from a team as a code owner August 14, 2026 07:30

melvin-bot Bot requested review from linhvovan29546 and removed request for a team August 14, 2026 07:30

Merge remote-tracking branch 'origin/main' into fix/93842-screens-rem…

e0af838

…oval-listener-uaf

This comment was marked as outdated.

Sign in to view

linhvovan29546 approved these changes Aug 18, 2026

View reviewed changes

melvin-bot Bot requested a review from mountiny August 18, 2026 08:59

exfy-clabot Bot added a commit to Expensify/CLA that referenced this pull request Aug 18, 2026

@mbSmaga has signed the CLA in Expensify/App#98632

e573c4a

This comment has been minimized.

Sign in to view

linhvovan29546 approved these changes Aug 19, 2026

View reviewed changes

mountiny approved these changes Aug 19, 2026

View reviewed changes

mountiny merged commit 2b031be into Expensify:main Aug 19, 2026
28 of 32 checks passed

Uh oh!

Conversation

mbSmaga commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Explanation of Change

Fixed Issues

Platform coverage and HybridApp

Tests

Offline tests

QA Steps

PR Author Checklist

Screenshots/Videos

Uh oh!

github-actions Bot commented Aug 14, 2026

Uh oh!

github-actions Bot commented Aug 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mbSmaga commented Aug 14, 2026

Uh oh!

mbSmaga commented Aug 14, 2026

Uh oh!

mbSmaga commented Aug 14, 2026

Uh oh!

melvin-bot Bot commented Aug 14, 2026

Uh oh!

linhvovan29546 commented Aug 17, 2026

Uh oh!

mbSmaga commented Aug 17, 2026

Uh oh!

linhvovan29546 commented Aug 18, 2026

Uh oh!

This comment was marked as outdated.

linhvovan29546 commented Aug 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer Checklist

Screenshots/Videos

Uh oh!

linhvovan29546 commented Aug 18, 2026

Uh oh!

linhvovan29546 left a comment

Choose a reason for hiding this comment

Uh oh!

mountiny commented Aug 18, 2026

Uh oh!

github-actions Bot commented Aug 18, 2026

Uh oh!

This comment has been minimized.

mountiny left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions Bot commented Aug 19, 2026

Uh oh!

OSBotify commented Aug 19, 2026

Uh oh!

github-actions Bot commented Aug 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

mbSmaga commented Aug 14, 2026 •

edited

Loading

github-actions Bot commented Aug 14, 2026 •

edited

Loading

linhvovan29546 commented Aug 18, 2026 •

edited

Loading