@samitouri / QOS-React / commits / 9ee7964302

Fix escaping in ReactDOMInput code (#26630)

JSON.stringify isn't the right thing here. Luckily this doesn't look to have any security impact.

Sophie Alpert committed Apr 24, 2023 at 10:33 UTC 9ee796430278c6f6e8acec4f54fd9be7f7868c0d
3 files changed +34 -19
packages/react-dom-bindings/src/client/ReactDOMInput.js
+4 -1
@@ -18,6 +18,7 @@ import {disableInputAttributeSyncing} from 'shared/ReactFeatureFlags';
18 import {checkAttributeStringCoercion} from 'shared/CheckStringCoercion';
19
20 import type {ToStringValue} from './ToStringValue';
21 +import escapeSelectorAttributeValueInsideDoubleQuotes from './escapeSelectorAttributeValueInsideDoubleQuotes';
22
23 let didWarnValueDefaultValue = false;
24 let didWarnCheckedDefaultChecked = false;
@@ -364,7 +365,9 @@ export function restoreControlledInputState(element: Element, props: Object) {
365 checkAttributeStringCoercion(name, 'name');
366 }
367 const group = queryRoot.querySelectorAll(
367 - 'input[name=' + JSON.stringify('' + name) + '][type="radio"]',
368 + 'input[name="' +
369 + escapeSelectorAttributeValueInsideDoubleQuotes('' + name) +
370 + '"][type="radio"]',
371 );
372
373 for (let i = 0; i < group.length; i++) {
packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js
+7 -18
@@ -102,6 +102,7 @@ import {
102 getValueDescriptorExpectingObjectForWarning,
103 getValueDescriptorExpectingEnumForWarning,
104 } from '../shared/ReactDOMResourceValidation';
105 +import escapeSelectorAttributeValueInsideDoubleQuotes from './escapeSelectorAttributeValueInsideDoubleQuotes';
106
107 export type Type = string;
108 export type Props = {
@@ -2478,11 +2479,13 @@ function styleTagPropsFromRawProps(
2479 function getStyleKey(href: string) {
2480 const limitedEscapedHref =
2481 escapeSelectorAttributeValueInsideDoubleQuotes(href);
2481 - return `href~="${limitedEscapedHref}"`;
2482 + return `href="${limitedEscapedHref}"`;
2483 }
2484
2484 -function getStyleTagSelectorFromKey(key: string) {
2485 - return `style[data-${key}]`;
2485 +function getStyleTagSelector(href: string) {
2486 + const limitedEscapedHref =
2487 + escapeSelectorAttributeValueInsideDoubleQuotes(href);
2488 + return `style[data-href~="${limitedEscapedHref}"]`;
2489 }
2490
2491 function getStylesheetSelectorFromKey(key: string) {
@@ -2567,11 +2570,10 @@ export function acquireResource(
2570 switch (resource.type) {
2571 case 'style': {
2572 const qualifiedProps: StyleTagQualifyingProps = props;
2570 - const key = getStyleKey(qualifiedProps.href);
2573
2574 // Attempt to hydrate instance from DOM
2575 let instance: null | Instance = hoistableRoot.querySelector(
2574 - getStyleTagSelectorFromKey(key),
2576 + getStyleTagSelector(qualifiedProps.href),
2577 );
2578 if (instance) {
2579 resource.instance = instance;
@@ -2952,19 +2954,6 @@ export function unmountHoistable(instance: Instance): void {
2954 (instance.parentNode: any).removeChild(instance);
2955 }
2956
2955 -// When passing user input into querySelector(All) the embedded string must not alter
2956 -// the semantics of the query. This escape function is safe to use when we know the
2957 -// provided value is going to be wrapped in double quotes as part of an attribute selector
2958 -// Do not use it anywhere else
2959 -// we escape double quotes and backslashes
2960 -const escapeSelectorAttributeValueInsideDoubleQuotesRegex = /[\n\"\\]/g;
2961 -function escapeSelectorAttributeValueInsideDoubleQuotes(value: string): string {
2962 - return value.replace(
2963 - escapeSelectorAttributeValueInsideDoubleQuotesRegex,
2964 - ch => '\\' + ch.charCodeAt(0).toString(16),
2965 - );
2966 -}
2967 -
2957 export function isHostHoistableType(
2958 type: string,
2959 props: RawProps,
packages/react-dom-bindings/src/client/escapeSelectorAttributeValueInsideDoubleQuotes.js new
+23
@@ -0,0 +1,23 @@
1 +/**
2 + * Copyright (c) Meta Platforms, Inc. and affiliates.
3 + *
4 + * This source code is licensed under the MIT license found in the
5 + * LICENSE file in the root directory of this source tree.
6 + *
7 + * @flow
8 + */
9 +
10 +// When passing user input into querySelector(All) the embedded string must not alter
11 +// the semantics of the query. This escape function is safe to use when we know the
12 +// provided value is going to be wrapped in double quotes as part of an attribute selector
13 +// Do not use it anywhere else
14 +// we escape double quotes and backslashes
15 +const escapeSelectorAttributeValueInsideDoubleQuotesRegex = /[\n\"\\]/g;
16 +export default function escapeSelectorAttributeValueInsideDoubleQuotes(
17 + value: string,
18 +): string {
19 + return value.replace(
20 + escapeSelectorAttributeValueInsideDoubleQuotesRegex,
21 + ch => '\\' + ch.charCodeAt(0).toString(16) + ' ',
22 + );
23 +}