fix(shared): wrap addObjectToProperties in try-catch to handle cross-origin SecurityError#36451
Open
sleitor wants to merge 1 commit into
Open
fix(shared): wrap addObjectToProperties in try-catch to handle cross-origin SecurityError#36451sleitor wants to merge 1 commit into
sleitor wants to merge 1 commit into
Conversation
…origin SecurityError In React 19.2 DEV builds, logComponentRender (called from commitPassiveMountOnFiber) calls addObjectToProperties which performs an unguarded 'for...in' over every prop value. When a component receives a cross-origin Window object (e.g. iframe.contentWindow with srcdoc='' / null origin) as a prop, the enumeration throws SecurityError in browsers that enforce the same-origin policy. This SecurityError propagates out of the commit phase and leaves the work-in-progress fiber broken, causing all subsequent renders to throw 'Should not already be working.' and making the UI completely unresponsive. Fix: - Wrap the 'for...in' loop in addObjectToProperties with try-catch. On SecurityError, show '[CrossOriginObject]' as a placeholder so the render-logger degrades gracefully without corrupting the fiber. - Wrap the 'in' / hasOwnProperty checks in readReactElementTypeof with try-catch so 2795888typeof access never escapes either. Both are DEV-only diagnostic paths and must not affect production builds or corrupt fiber state when iterating untrusted props. Repro: pass iframe.contentWindow (srcdoc iframe, null origin) as a prop to any component in a DEV Vite+React 19.2 app. Fixes facebook#36430
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
In React 19.2 DEV builds,
logComponentRender(called fromcommitPassiveMountOnFiber) callsaddObjectToPropertieswhich performs an unguardedfor...inover every prop value. When a component receives a cross-originWindowobject (e.g.iframe.contentWindowwithsrcdoc=""/ null origin) as a prop, the enumeration throwsSecurityErrorin browsers that enforce the same-origin policy.This
SecurityErrorpropagates out of the commit phase and leaves the work-in-progress fiber broken, causing all subsequent renders to throwShould not already be working.and making the UI completely unresponsive.Root Cause
addObjectToPropertiesinpackages/shared/ReactPerformanceTrackProperties.jsrunsfor (const key in object)without any error guard. For a cross-originWindow(null origin iframe), browsers block property enumeration and throwSecurityError. Since this error is not caught, it escapes the commit phase and corrupts the fiber tree.Similarly,
readReactElementTypeofaccesses'2796107typeof' in valuewithout a guard, which can also throw for cross-origin objects.Fix
for...inloop inaddObjectToPropertieswith try-catch. On SecurityError, show[CrossOriginObject]as a placeholder so the render-logger degrades gracefully without corrupting the fiber.in/ hasOwnProperty checks inreadReactElementTypeofwith try-catch so$$typeofaccess never escapes either.Both are DEV-only diagnostic paths and must not affect production builds or corrupt fiber state when iterating untrusted props.
Test
Added a new test in
ReactPerformanceTrack-test.jsthat simulates a cross-originWindowvia a Proxy whoseownKeysandgetOwnPropertyDescriptortraps throw, verifying that:Fixes #36430