Skip to content

fix(LayoutAnimations): don't read ViewProps opacity from nested Text props - #10732

Open
hirvesh wants to merge 1 commit into
software-mansion:mainfrom
hirvesh:fix/nested-text-entering-opacity
Open

hirvesh wants to merge 1 commit into
software-mansion:mainfrom
hirvesh:fix/nested-text-entering-opacity

Conversation

@hirvesh

@hirvesh hirvesh commented Sep 26, 2026

Copy link
Copy Markdown

Note

This pull request was authored by AI on behalf of @hirvesh.

Summary

LayoutAnimationsProxyCommon::updateEnteringAnimationTarget reads opacity by casting the view's props to ViewProps. It does this before it checks for an entering animation:

const auto opacity = static_cast<const ViewProps &>(*finalView.props).opacity;

Both LayoutAnimationsProxy::updateLightTree and LayoutAnimationsProxy_Legacy call this function for every Update mutation that has no layout config. USE_LEGACY_LAYOUT_ANIMATIONS_PROXY therefore does not avoid it.

On Android, TextShadowNode sets ShadowNodeTraits::Trait::FormsView. A <Text> nested in another <Text> therefore gets mount mutations, and its props are TextProps. TextProps derives from Props, not ViewProps. It is also smaller than the offset of ViewProps::opacity: in an arm64 release build on RN 0.88, sizeof(TextProps) is 0x148 and opacity is at offset 0x2b0. So on every such update, the read lands 360 bytes past the end of the TextProps allocation.

With the default allocator this usually returns garbage and goes unnoticed. With allocators that guard the memory after an allocation, it crashes. We see this in production on 4.7.0 as a native SIGSEGV at LayoutAnimationsProxyCommon.cpp:509:

  • occasionally on Pixel, Samsung and Xiaomi devices on Android 15–17;
  • on every launch on a Pixel 10 running GrapheneOS, which uses hardened_malloc.
signal 11 (SIGSEGV), code 2 (SEGV_ACCERR), fault addr 0x...148 (read)
#00 pc 0000000000149bf4  libreanimated.so  (LayoutAnimationsProxyCommon::updateEnteringAnimationTarget, LayoutAnimationsProxyCommon.cpp:509)
#01 pc 0000000000137104  libreanimated.so  (LayoutAnimationsProxy::updateLightTree)
#02 pc 0000000000134d70  libreanimated.so  (LayoutAnimationsProxy::pullTransaction)
#03 pc 00000000004fa798  libreactnative.so (facebook::react::MountingCoordinator::pullTransaction(bool) const+1668)

This PR reads opacity only from ViewKind nodes. That is the check React Native's LayoutAnimationKeyFrameManager makes before it casts props to ViewProps. For other nodes, opacity is std::nullopt. Both fields it is assigned to are already std::optional<double>. The existing lookups and assignments are unchanged, so views behave as before.

Other ViewProps casts on the entering and synchronous-props paths remain, for example LayoutAnimationsProxyCommon.cpp:207 and :229. They only run for nodes that already have a layout animation, so they do not have this exposure. I left them unchanged, but I'm happy to guard them too if you prefer.

Test plan

I reproduced the crash in a production app, not in a minimal example. GWP-ASan attributes the faulting TextProps allocation to reanimated::mergeProps in ShadowTreeCloner.cpp, so the trigger is a nested <Text> whose props Reanimated updates. The snippet below should exercise the same path, but I have not verified it on its own:

function NestedAnimatedText() {
  const progress = useSharedValue(0);
  useEffect(() => {
    progress.value = withRepeat(withTiming(1, { duration: 500 }), -1, true);
  }, []);
  const style = useAnimatedStyle(() => ({
    color: interpolateColor(progress.value, [0, 1], ['red', 'blue']),
  }));
  return (
    <Text>
      Outer <Animated.Text style={style}>inner</Animated.Text>
    </Text>
  );
}

The default allocator does not fault on this read, so the crash needs GWP-ASan with every sampled allocation guarded. Run this on a rootable arm64 emulator (a google_apis image, API 36):

adb root
P=<applicationId>
adb shell setprop libc.debug.gwp_asan.sample_rate.$P 5
adb shell setprop libc.debug.gwp_asan.process_sampling.$P 1
adb shell setprop libc.debug.gwp_asan.max_allocs.$P 30000
adb shell setprop libc.debug.gwp_asan.recoverable.$P false
# cold-launch a release build: adb shell am force-stop $P && adb shell am start -W -n $P/.MainActivity

Results, with react-native-reanimated 4.7.0, react-native-worklets 0.13.0 and RN 0.88.0-rc.1 (New Architecture, arm64 release builds):

  • Without this change: 3 of 3 cold launches crashed. GWP-ASan reported Buffer Overflow, 360 bytes right of a 352-byte allocation in updateEnteringAnimationTarget, called from updateLightTree. The 352-byte allocation is the make_shared<TextProps> block, and the reported offset is exactly where ViewProps::opacity is read.
  • With this change: 15 of 15 cold launches ran without a crash or GWP-ASan report. In 10 of those runs, temporary logging showed the non-ViewKind branch running 14–34 times per launch, always for component=Text, where it previously read out of bounds.

This file is byte-identical on main and in 4.7.0, so the change applies unchanged.

Changelog

  • I added a changelog fragment with yarn changelog:add for each changed package, or this PR does not change react-native-reanimated or react-native-worklets.

…props

updateEnteringAnimationTarget runs for every Update mutation without a
layout config and cast the view's props to ViewProps before checking for
an entering animation. On Android, a <Text> nested in another <Text>
forms a view (TextShadowNode sets FormsView) whose props are TextProps,
which does not derive from ViewProps and is smaller than the offset of
ViewProps::opacity. The read went past the end of the TextProps
allocation and crashed with SIGSEGV on allocators that guard it
(GrapheneOS hardened_malloc, GWP-ASan).

Only read opacity from ViewKind nodes, the same check React Native's
LayoutAnimationKeyFrameManager uses before casting to ViewProps.
@coderabbitai

coderabbitai Bot commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 83c5bac0-4e21-4345-9847-ec7508fd5082

📥 Commits

Reviewing files that changed from the base of the PR and between 9b3ae12 and e7816c9.

📒 Files selected for processing (2)
  • packages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/LayoutAnimationsProxyCommon.cpp
  • packages/react-native-reanimated/changelog/nested-text-entering-opacity.fix.md

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

updateEnteringAnimationTarget now reads opacity from ViewProps only when the node has the ViewKind trait. For other node kinds, it leaves opacity unset. The changelog describes an Android native crash involving nested text opacity.

Priority: ➖ Normal

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to e7816

The change avoids reading view opacity from non-view nodes, and the supplied downstream behavior safely ignores unset opacity. No concrete merge-blocking risk is evident.

Architecture Summary

Architecture risk: 🔵 Low · up to e7816

The change affects 1 system.

Changed systems: packages/react-native-reanimated

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — packages/react-native-reanimated (library) was modified; 2 changed files map to changed impact.

Before / after behavior

  • observed — Modified behavior in packages/react-native-reanimated/Common/cpp/reanimated/LayoutAnimations/LayoutAnimationsProxyCommon.cpp: updateEnteringAnimationTarget no longer unconditionally casts finalView.props to ViewProps. It reads opacity only for ViewKind nodes and leaves it unset for other node kinds.
  • observed — Modified behavior in packages/react-native-reanimated/changelog/nested-text-entering-opacity.fix.md: Added a changelog note about an Android crash caused by treating opacity on a nested <Text> as a view prop.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main fix: preventing ViewProps opacity reads from nested Text props during layout animations.
Description check ✅ Passed The description directly explains the out-of-bounds read, affected nested Text nodes, the code change, test results, and changelog entry.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant