From ee2e8583b0dfd89da0a5d491d89a3f25967e2a64 Mon Sep 17 00:00:00 2001 From: Aarush Dubey Date: Fri, 23 Jan 2026 16:49:13 +0530 Subject: [PATCH] Fix ReDoS in Firefox stack parser (#35490) Replace nested quantifiers with negated character class to prevent catastrophic backtracking. Changes (?:.*\".+\")? to (?:\"[^\"]\")? Fixes #35490 --- .../src/__tests__/utils-test.js | 88 +++++++++++-------- .../src/backend/utils/parseStackTrace.js | 4 +- 2 files changed, 52 insertions(+), 40 deletions(-) diff --git a/packages/react-devtools-shared/src/__tests__/utils-test.js b/packages/react-devtools-shared/src/__tests__/utils-test.js index 9b9e82102cc..a498391ab92 100644 --- a/packages/react-devtools-shared/src/__tests__/utils-test.js +++ b/packages/react-devtools-shared/src/__tests__/utils-test.js @@ -12,7 +12,7 @@ import { getDisplayNameForReactElement, isPlainObject, } from 'react-devtools-shared/src/utils'; -import {stackToComponentLocations} from 'react-devtools-shared/src/devtools/utils'; +import { stackToComponentLocations } from 'react-devtools-shared/src/devtools/utils'; import { formatConsoleArguments, formatConsoleArgumentsToSingleString, @@ -20,44 +20,44 @@ import { gt, gte, } from 'react-devtools-shared/src/backend/utils'; -import {extractLocationFromComponentStack} from 'react-devtools-shared/src/backend/utils/parseStackTrace'; +import { extractLocationFromComponentStack } from 'react-devtools-shared/src/backend/utils/parseStackTrace'; import { REACT_SUSPENSE_LIST_TYPE as SuspenseList, REACT_STRICT_MODE_TYPE as StrictMode, } from 'shared/ReactSymbols'; -import {createElement} from 'react'; -import {symbolicateSource} from '../symbolicateSource'; +import { createElement } from 'react'; +import { symbolicateSource } from '../symbolicateSource'; describe('utils', () => { describe('getDisplayName', () => { // @reactVersion >= 16.0 it('should return a function name', () => { - function FauxComponent() {} + function FauxComponent() { } expect(getDisplayName(FauxComponent)).toEqual('FauxComponent'); }); // @reactVersion >= 16.0 it('should return a displayName name if specified', () => { - function FauxComponent() {} + function FauxComponent() { } FauxComponent.displayName = 'OverrideDisplayName'; expect(getDisplayName(FauxComponent)).toEqual('OverrideDisplayName'); }); // @reactVersion >= 16.0 it('should return the fallback for anonymous functions', () => { - expect(getDisplayName(() => {}, 'Fallback')).toEqual('Fallback'); + expect(getDisplayName(() => { }, 'Fallback')).toEqual('Fallback'); }); // @reactVersion >= 16.0 it('should return Anonymous for anonymous functions without a fallback', () => { - expect(getDisplayName(() => {})).toEqual('Anonymous'); + expect(getDisplayName(() => { })).toEqual('Anonymous'); }); // Simulate a reported bug: // https://github.com/facebook/react/issues/16685 // @reactVersion >= 16.0 it('should return a fallback when the name prop is not a string', () => { - const FauxComponent = {name: {}}; + const FauxComponent = { name: {} }; expect(getDisplayName(FauxComponent, 'Fallback')).toEqual('Fallback'); }); @@ -85,7 +85,7 @@ describe('utils', () => { describe('getDisplayNameForReactElement', () => { // @reactVersion >= 16.0 it('should return correct display name for an element with function type', () => { - function FauxComponent() {} + function FauxComponent() { } FauxComponent.displayName = 'OverrideDisplayName'; const element = createElement(FauxComponent); expect(getDisplayNameForReactElement(element)).toEqual( @@ -213,7 +213,7 @@ describe('utils', () => { const symbol = Symbol('a'); expect( formatWithStyles( - ['abc', 123, 12.3, true, {hello: 'world'}, symbol], + ['abc', 123, 12.3, true, { hello: 'world' }, symbol], 'color: gray', ), ).toEqual([ @@ -223,7 +223,7 @@ describe('utils', () => { 123, 12.3, true, - {hello: 'world'}, + { hello: 'world' }, symbol, ]); }); @@ -243,22 +243,22 @@ describe('utils', () => { }); it('should format non string inputs as the first argument', () => { - expect(formatWithStyles([{foo: 'bar'}])).toEqual([{foo: 'bar'}]); + expect(formatWithStyles([{ foo: 'bar' }])).toEqual([{ foo: 'bar' }]); expect(formatWithStyles([[1, 2, 3]])).toEqual([[1, 2, 3]]); - expect(formatWithStyles([{foo: 'bar'}], 'color: gray')).toEqual([ + expect(formatWithStyles([{ foo: 'bar' }], 'color: gray')).toEqual([ '%c%o', 'color: gray', - {foo: 'bar'}, + { foo: 'bar' }, ]); expect(formatWithStyles([[1, 2, 3]], 'color: gray')).toEqual([ '%c%o', 'color: gray', [1, 2, 3], ]); - expect(formatWithStyles([{foo: 'bar'}, 'hi'], 'color: gray')).toEqual([ + expect(formatWithStyles([{ foo: 'bar' }, 'hi'], 'color: gray')).toEqual([ '%c%o %s', 'color: gray', - {foo: 'bar'}, + { foo: 'bar' }, 'hi', ]); }); @@ -283,12 +283,12 @@ describe('utils', () => { describe('isPlainObject', () => { it('should return true for plain objects', () => { expect(isPlainObject({})).toBe(true); - expect(isPlainObject({a: 1})).toBe(true); - expect(isPlainObject({a: {b: {c: 123}}})).toBe(true); + expect(isPlainObject({ a: 1 })).toBe(true); + expect(isPlainObject({ a: { b: { c: 123 } } })).toBe(true); }); it('should return false if object is a class instance', () => { - expect(isPlainObject(new (class C {})())).toBe(false); + expect(isPlainObject(new (class C { })())).toBe(false); }); it('should return false for objects, which have not only Object in its prototype chain', () => { @@ -315,8 +315,8 @@ describe('utils', () => { expect( extractLocationFromComponentStack( 'at l (https://react.dev/_next/static/chunks/main-78a3b4c2aa4e4850.js:1:10389)\n' + - 'at f (https://react.dev/_next/static/chunks/pages/%5B%5B...markdownPath%5D%5D-af2ed613aedf1d57.js:1:8519)\n' + - 'at r (https://react.dev/_next/static/chunks/pages/_app-dd0b77ea7bd5b246.js:1:498)\n', + 'at f (https://react.dev/_next/static/chunks/pages/%5B%5B...markdownPath%5D%5D-af2ed613aedf1d57.js:1:8519)\n' + + 'at r (https://react.dev/_next/static/chunks/pages/_app-dd0b77ea7bd5b246.js:1:498)\n', ), ).toEqual([ 'l', @@ -330,16 +330,16 @@ describe('utils', () => { expect( extractLocationFromComponentStack( ' at Q\n' + - ' at a\n' + - ' at m (https://react.dev/_next/static/chunks/848-122f91e9565d9ffa.js:5:9236)\n' + - ' at div\n' + - ' at div\n' + - ' at div\n' + - ' at nav\n' + - ' at div\n' + - ' at te (https://react.dev/_next/static/chunks/363-3c5f1b553b6be118.js:1:158857)\n' + - ' at tt (https://react.dev/_next/static/chunks/363-3c5f1b553b6be118.js:1:165520)\n' + - ' at f (https://react.dev/_next/static/chunks/pages/%5B%5B...markdownPath%5D%5D-af2ed613aedf1d57.js:1:8519)', + ' at a\n' + + ' at m (https://react.dev/_next/static/chunks/848-122f91e9565d9ffa.js:5:9236)\n' + + ' at div\n' + + ' at div\n' + + ' at div\n' + + ' at nav\n' + + ' at div\n' + + ' at te (https://react.dev/_next/static/chunks/363-3c5f1b553b6be118.js:1:158857)\n' + + ' at tt (https://react.dev/_next/static/chunks/363-3c5f1b553b6be118.js:1:165520)\n' + + ' at f (https://react.dev/_next/static/chunks/pages/%5B%5B...markdownPath%5D%5D-af2ed613aedf1d57.js:1:8519)', ), ).toEqual([ 'm', @@ -353,8 +353,8 @@ describe('utils', () => { expect( extractLocationFromComponentStack( ' at Q\n' + - ' at a\n' + - ' at https://react.dev/_next/static/chunks/848-122f91e9565d9ffa.js:5:9236\n', + ' at a\n' + + ' at https://react.dev/_next/static/chunks/848-122f91e9565d9ffa.js:5:9236\n', ), ).toEqual([ '', @@ -368,8 +368,8 @@ describe('utils', () => { expect( extractLocationFromComponentStack( 'at HotReload (webpack-internal:///(app-pages-browser)/./node_modules/next/dist/client/components/react-dev-overlay/hot-reloader-client.js:307:11)\n' + - ' at Router (webpack-internal:///(app-pages-browser)/./node_modules/next/dist/client/components/app-router.js:181:11)\n' + - ' at ErrorBoundaryHandler (webpack-internal:///(app-pages-browser)/./node_modules/next/dist/client/components/error-boundary.js:114:9)', + ' at Router (webpack-internal:///(app-pages-browser)/./node_modules/next/dist/client/components/app-router.js:181:11)\n' + + ' at ErrorBoundaryHandler (webpack-internal:///(app-pages-browser)/./node_modules/next/dist/client/components/error-boundary.js:114:9)', ), ).toEqual([ 'HotReload', @@ -383,8 +383,8 @@ describe('utils', () => { expect( extractLocationFromComponentStack( 'tt@https://react.dev/_next/static/chunks/363-3c5f1b553b6be118.js:1:165558\n' + - 'f@https://react.dev/_next/static/chunks/pages/%5B%5B...markdownPath%5D%5D-af2ed613aedf1d57.js:1:8535\n' + - 'r@https://react.dev/_next/static/chunks/pages/_app-dd0b77ea7bd5b246.js:1:513', + 'f@https://react.dev/_next/static/chunks/pages/%5B%5B...markdownPath%5D%5D-af2ed613aedf1d57.js:1:8535\n' + + 'r@https://react.dev/_next/static/chunks/pages/_app-dd0b77ea7bd5b246.js:1:513', ), ).toEqual([ 'tt', @@ -393,6 +393,18 @@ describe('utils', () => { 165558, ]); }); + + // Regression test for #35490 + it('should not hang on malicious input', () => { + const maliciousInput = ' ' + ('"\0').repeat(2000) + '\r!\r!'; + + const startTime = Date.now(); + const result = extractLocationFromComponentStack(maliciousInput); + const duration = Date.now() - startTime; + + expect(duration).toBeLessThan(100); + expect(result).toEqual(null); + }); }); describe('symbolicateSource', () => { diff --git a/packages/react-devtools-shared/src/backend/utils/parseStackTrace.js b/packages/react-devtools-shared/src/backend/utils/parseStackTrace.js index e18c2d7baa1..1aab8603487 100644 --- a/packages/react-devtools-shared/src/backend/utils/parseStackTrace.js +++ b/packages/react-devtools-shared/src/backend/utils/parseStackTrace.js @@ -8,7 +8,7 @@ * @flow */ -import type {ReactStackTrace, ReactFunctionLocation} from 'shared/ReactTypes'; +import type { ReactStackTrace, ReactFunctionLocation } from 'shared/ReactTypes'; function parseStackTraceFromChromeStack( stack: string, @@ -59,7 +59,7 @@ function parseStackTraceFromChromeStack( return parsedFrames; } -const firefoxFrameRegExp = /^((?:.*".+")?[^@]*)@(.+):(\d+):(\d+)$/; +const firefoxFrameRegExp = /^((?:"[^"]+")?[^@]*)@(.+):(\d+):(\d+)$/; function parseStackTraceFromFirefoxStack( stack: string, skipFrames: number,