From 810c2051ee0eaa0e9d69de1f09d2e50835ebe25e Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Thu, 1 Feb 2024 00:33:54 -0800 Subject: [PATCH] Fix "swapLeftAndRight" unsafe static_cast (#42721) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/42721 If left and right are swapped, this code assumes every yoga layoutable shadownode is a view, and mutates its props as if they were ViewProps. This is not safe, and could lead to memory corruption. Changelog: [Internal] Reviewed By: rozele Differential Revision: D53213652 fbshipit-source-id: c43e0f80fdd5889761317c1243ccc0ab392e3443 --- .../view/YogaLayoutableShadowNode.cpp | 108 +++++++++--------- .../view/YogaLayoutableShadowNode.h | 6 +- 2 files changed, 55 insertions(+), 59 deletions(-) diff --git a/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp b/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp index 9e655f5fdf0..1361692e034 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp +++ b/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp @@ -863,13 +863,12 @@ yoga::Config& YogaLayoutableShadowNode::initializeYogaConfig( void YogaLayoutableShadowNode::swapStyleLeftAndRight() { ensureUnsealed(); - swapLeftAndRightInYogaStyleProps(*this); - swapLeftAndRightInViewProps(*this); + swapLeftAndRightInYogaStyleProps(); + swapLeftAndRightInViewProps(); } -void YogaLayoutableShadowNode::swapLeftAndRightInYogaStyleProps( - const YogaLayoutableShadowNode& shadowNode) { - auto yogaStyle = shadowNode.yogaNode_.style(); +void YogaLayoutableShadowNode::swapLeftAndRightInYogaStyleProps() { + auto yogaStyle = yogaNode_.style(); // Swap Yoga node values, position, padding and margin. @@ -906,66 +905,65 @@ void YogaLayoutableShadowNode::swapLeftAndRightInYogaStyleProps( yogaStyle.setMargin(yoga::Edge::Right, yoga::value::undefined()); } - shadowNode.yogaNode_.setStyle(yogaStyle); + if (yogaStyle.border(yoga::Edge::Left).isDefined()) { + yogaStyle.setBorder(yoga::Edge::Start, yogaStyle.border(yoga::Edge::Left)); + yogaStyle.setBorder(yoga::Edge::Left, yoga::value::undefined()); + } + + if (yogaStyle.border(yoga::Edge::Right).isDefined()) { + yogaStyle.setBorder(yoga::Edge::End, yogaStyle.border(yoga::Edge::Right)); + yogaStyle.setBorder(yoga::Edge::Right, yoga::value::undefined()); + } + + yogaNode_.setStyle(yogaStyle); } -void YogaLayoutableShadowNode::swapLeftAndRightInViewProps( - const YogaLayoutableShadowNode& shadowNode) { - auto& typedCasting = static_cast(*shadowNode.props_); - auto& props = const_cast(typedCasting); +void YogaLayoutableShadowNode::swapLeftAndRightInViewProps() { + if (auto viewShadowNode = dynamic_cast(this)) { + // TODO: Do not mutate props directly. + auto& props = + const_cast(viewShadowNode->getConcreteProps()); - // Swap border node values, borderRadii, borderColors and borderStyles. + // Swap border node values, borderRadii, borderColors and borderStyles. + if (props.borderRadii.topLeft.has_value()) { + props.borderRadii.topStart = props.borderRadii.topLeft; + props.borderRadii.topLeft.reset(); + } - if (props.borderRadii.topLeft.has_value()) { - props.borderRadii.topStart = props.borderRadii.topLeft; - props.borderRadii.topLeft.reset(); - } + if (props.borderRadii.bottomLeft.has_value()) { + props.borderRadii.bottomStart = props.borderRadii.bottomLeft; + props.borderRadii.bottomLeft.reset(); + } - if (props.borderRadii.bottomLeft.has_value()) { - props.borderRadii.bottomStart = props.borderRadii.bottomLeft; - props.borderRadii.bottomLeft.reset(); - } + if (props.borderRadii.topRight.has_value()) { + props.borderRadii.topEnd = props.borderRadii.topRight; + props.borderRadii.topRight.reset(); + } - if (props.borderRadii.topRight.has_value()) { - props.borderRadii.topEnd = props.borderRadii.topRight; - props.borderRadii.topRight.reset(); - } + if (props.borderRadii.bottomRight.has_value()) { + props.borderRadii.bottomEnd = props.borderRadii.bottomRight; + props.borderRadii.bottomRight.reset(); + } - if (props.borderRadii.bottomRight.has_value()) { - props.borderRadii.bottomEnd = props.borderRadii.bottomRight; - props.borderRadii.bottomRight.reset(); - } + if (props.borderColors.left.has_value()) { + props.borderColors.start = props.borderColors.left; + props.borderColors.left.reset(); + } - if (props.borderColors.left.has_value()) { - props.borderColors.start = props.borderColors.left; - props.borderColors.left.reset(); - } + if (props.borderColors.right.has_value()) { + props.borderColors.end = props.borderColors.right; + props.borderColors.right.reset(); + } - if (props.borderColors.right.has_value()) { - props.borderColors.end = props.borderColors.right; - props.borderColors.right.reset(); - } + if (props.borderStyles.left.has_value()) { + props.borderStyles.start = props.borderStyles.left; + props.borderStyles.left.reset(); + } - if (props.borderStyles.left.has_value()) { - props.borderStyles.start = props.borderStyles.left; - props.borderStyles.left.reset(); - } - - if (props.borderStyles.right.has_value()) { - props.borderStyles.end = props.borderStyles.right; - props.borderStyles.right.reset(); - } - - if (props.yogaStyle.border(yoga::Edge::Left).isDefined()) { - props.yogaStyle.setBorder( - yoga::Edge::Start, props.yogaStyle.border(yoga::Edge::Left)); - props.yogaStyle.setBorder(yoga::Edge::Left, yoga::value::undefined()); - } - - if (props.yogaStyle.border(yoga::Edge::Right).isDefined()) { - props.yogaStyle.setBorder( - yoga::Edge::End, props.yogaStyle.border(yoga::Edge::Right)); - props.yogaStyle.setBorder(yoga::Edge::Right, yoga::value::undefined()); + if (props.borderStyles.right.has_value()) { + props.borderStyles.end = props.borderStyles.right; + props.borderStyles.right.reset(); + } } } diff --git a/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.h b/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.h index f4a9ba79436..d924d7366e9 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.h +++ b/packages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.h @@ -190,16 +190,14 @@ class YogaLayoutableShadowNode : public LayoutableShadowNode { * - border(Left|Right)Width → border(Start|End)Width * - border(Left|Right)Color → border(Start|End)Color */ - static void swapLeftAndRightInViewProps( - const YogaLayoutableShadowNode& shadowNode); + void swapLeftAndRightInViewProps(); /* * In yoga node passed as argument, reassigns following values * - (left|right) → (start|end) * - margin(Left|Right) → margin(Start|End) * - padding(Left|Right) → padding(Start|End) */ - static void swapLeftAndRightInYogaStyleProps( - const YogaLayoutableShadowNode& shadowNode); + void swapLeftAndRightInYogaStyleProps(); /* * Combine a base yoga::Style with aliased properties which should be