[skip ci] Fix "swapLeftAndRight" unsafe static_cast (#42721)

Summary: 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] Differential Revision: D53213652
facebook · Jan 30, 2024 · e6e3665 · e6e3665
1 parent c7479b0
commit e6e3665
Show file tree

Hide file tree

Showing 2 changed files with 54 additions and 58 deletions.
diff --git a/...ages/react-native/ReactCommon/react/renderer/components/view/YogaLayoutableShadowNode.cpp b/...ages/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);
-}
-
-void YogaLayoutableShadowNode::swapLeftAndRightInViewProps(
-    const YogaLayoutableShadowNode& shadowNode) {
-  auto& typedCasting = static_cast<const ViewProps&>(*shadowNode.props_);
-  auto& props = const_cast<ViewProps&>(typedCasting);
-
-  // 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 (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 (props.borderRadii.bottomLeft.has_value()) {
-    props.borderRadii.bottomStart = props.borderRadii.bottomLeft;
-    props.borderRadii.bottomLeft.reset();
+  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());
   }
 
-  if (props.borderRadii.topRight.has_value()) {
-    props.borderRadii.topEnd = props.borderRadii.topRight;
-    props.borderRadii.topRight.reset();
-  }
+  yogaNode_.setStyle(yogaStyle);
+}
 
-  if (props.borderRadii.bottomRight.has_value()) {
-    props.borderRadii.bottomEnd = props.borderRadii.bottomRight;
-    props.borderRadii.bottomRight.reset();
-  }
+void YogaLayoutableShadowNode::swapLeftAndRightInViewProps() {
+  if (auto viewShadowNode = dynamic_cast<ViewShadowNode*>(this)) {
+    // TODO: Do not mutate props directly.
+    auto& props =
+        const_cast<ViewShadowNodeProps&>(viewShadowNode->getConcreteProps());
 
-  if (props.borderColors.left.has_value()) {
-    props.borderColors.start = props.borderColors.left;
-    props.borderColors.left.reset();
-  }
+    // 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.borderColors.right.has_value()) {
-    props.borderColors.end = props.borderColors.right;
-    props.borderColors.right.reset();
-  }
+    if (props.borderRadii.bottomLeft.has_value()) {
+      props.borderRadii.bottomStart = props.borderRadii.bottomLeft;
+      props.borderRadii.bottomLeft.reset();
+    }
 
-  if (props.borderStyles.left.has_value()) {
-    props.borderStyles.start = props.borderStyles.left;
-    props.borderStyles.left.reset();
-  }
+    if (props.borderRadii.topRight.has_value()) {
+      props.borderRadii.topEnd = props.borderRadii.topRight;
+      props.borderRadii.topRight.reset();
+    }
 
-  if (props.borderStyles.right.has_value()) {
-    props.borderStyles.end = props.borderStyles.right;
-    props.borderStyles.right.reset();
-  }
+    if (props.borderRadii.bottomRight.has_value()) {
+      props.borderRadii.bottomEnd = props.borderRadii.bottomRight;
+      props.borderRadii.bottomRight.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.borderColors.left.has_value()) {
+      props.borderColors.start = props.borderColors.left;
+      props.borderColors.left.reset();
+    }
 
-  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.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.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
@@ -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