@samitouri / QOS-React-1 / commits / e61a60fac0

[Flight] Enforce "simple object" rule in production (#27502)

We only allow plain objects that can be faithfully serialized and deserialized through JSON to pass through the serialization boundary. It's a bit too expensive to do all the possible checks in production so we do most checks in DEV, so it's still possible to pass an object in production by mistake. This is currently exaggerated by frameworks because the logs on the server aren't visible enough. Even so, it's possible to do a mistake without testing it in DEV or just testing a conditional branch. That might have security implications if that object wasn't supposed to be passed. We can't rely on only checking if the prototype is `Object.prototype` because that wouldn't work with cross-realm objects which is unfortunate. However, if it isn't, we can check wether it has exactly one prototype on the chain which would catch the common error of passing a class instance.

Sebastian Markbåge committed Oct 11, 2023 at 12:18 UTC e61a60fac02d205ad928bff6de2449f00646a92c
7 files changed +127 -83
packages/react-client/src/ReactFlightReplyClient.js
+54 -41
@@ -29,6 +29,9 @@ import {
29 } from 'shared/ReactSerializationErrors';
30
31 import isArray from 'shared/isArray';
32 +import getPrototypeOf from 'shared/getPrototypeOf';
33 +
34 +const ObjectPrototype = Object.prototype;
35
36 import {usedWithSSR} from './ReactFlightClientConfig';
37
@@ -227,6 +230,10 @@ export function processReply(
230 );
231 return serializePromiseID(promiseId);
232 }
233 + if (isArray(value)) {
234 + // $FlowFixMe[incompatible-return]
235 + return value;
236 + }
237 // TODO: Should we the Object.prototype.toString.call() to test for cross-realm objects?
238 if (value instanceof FormData) {
239 if (formData === null) {
@@ -263,54 +270,60 @@ export function processReply(
270 formData.append(formFieldPrefix + setId, partJSON);
271 return serializeSetID(setId);
272 }
266 - if (!isArray(value)) {
267 - const iteratorFn = getIteratorFn(value);
268 - if (iteratorFn) {
269 - return Array.from((value: any));
270 - }
273 + const iteratorFn = getIteratorFn(value);
274 + if (iteratorFn) {
275 + return Array.from((value: any));
276 }
277
278 + // Verify that this is a simple plain object.
279 + const proto = getPrototypeOf(value);
280 + if (
281 + proto !== ObjectPrototype &&
282 + (proto === null || getPrototypeOf(proto) !== null)
283 + ) {
284 + throw new Error(
285 + 'Only plain objects, and a few built-ins, can be passed to Server Actions. ' +
286 + 'Classes or null prototypes are not supported.',
287 + );
288 + }
289 if (__DEV__) {
274 - if (value !== null && !isArray(value)) {
275 - // Verify that this is a simple plain object.
276 - if ((value: any).$$typeof === REACT_ELEMENT_TYPE) {
277 - console.error(
278 - 'React Element cannot be passed to Server Functions from the Client.%s',
279 - describeObjectForErrorMessage(parent, key),
280 - );
281 - } else if ((value: any).$$typeof === REACT_LAZY_TYPE) {
282 - console.error(
283 - 'React Lazy cannot be passed to Server Functions from the Client.%s',
284 - describeObjectForErrorMessage(parent, key),
285 - );
286 - } else if ((value: any).$$typeof === REACT_PROVIDER_TYPE) {
287 - console.error(
288 - 'React Context Providers cannot be passed to Server Functions from the Client.%s',
289 - describeObjectForErrorMessage(parent, key),
290 - );
291 - } else if (objectName(value) !== 'Object') {
292 - console.error(
293 - 'Only plain objects can be passed to Client Components from Server Components. ' +
294 - '%s objects are not supported.%s',
295 - objectName(value),
296 - describeObjectForErrorMessage(parent, key),
297 - );
298 - } else if (!isSimpleObject(value)) {
290 + if ((value: any).$$typeof === REACT_ELEMENT_TYPE) {
291 + console.error(
292 + 'React Element cannot be passed to Server Functions from the Client.%s',
293 + describeObjectForErrorMessage(parent, key),
294 + );
295 + } else if ((value: any).$$typeof === REACT_LAZY_TYPE) {
296 + console.error(
297 + 'React Lazy cannot be passed to Server Functions from the Client.%s',
298 + describeObjectForErrorMessage(parent, key),
299 + );
300 + } else if ((value: any).$$typeof === REACT_PROVIDER_TYPE) {
301 + console.error(
302 + 'React Context Providers cannot be passed to Server Functions from the Client.%s',
303 + describeObjectForErrorMessage(parent, key),
304 + );
305 + } else if (objectName(value) !== 'Object') {
306 + console.error(
307 + 'Only plain objects can be passed to Client Components from Server Components. ' +
308 + '%s objects are not supported.%s',
309 + objectName(value),
310 + describeObjectForErrorMessage(parent, key),
311 + );
312 + } else if (!isSimpleObject(value)) {
313 + console.error(
314 + 'Only plain objects can be passed to Client Components from Server Components. ' +
315 + 'Classes or other objects with methods are not supported.%s',
316 + describeObjectForErrorMessage(parent, key),
317 + );
318 + } else if (Object.getOwnPropertySymbols) {
319 + const symbols = Object.getOwnPropertySymbols(value);
320 + if (symbols.length > 0) {
321 console.error(
322 'Only plain objects can be passed to Client Components from Server Components. ' +
301 - 'Classes or other objects with methods are not supported.%s',
323 + 'Objects with symbol properties like %s are not supported.%s',
324 + symbols[0].description,
325 describeObjectForErrorMessage(parent, key),
326 );
304 - } else if (Object.getOwnPropertySymbols) {
305 - const symbols = Object.getOwnPropertySymbols(value);
306 - if (symbols.length > 0) {
307 - console.error(
308 - 'Only plain objects can be passed to Client Components from Server Components. ' +
309 - 'Objects with symbol properties like %s are not supported.%s',
310 - symbols[0].description,
311 - describeObjectForErrorMessage(parent, key),
312 - );
313 - }
327 }
328 }
329 }
packages/react-client/src/__tests__/ReactFlight-test.js
+12 -10
@@ -988,19 +988,21 @@ describe('ReactFlight', () => {
988 ReactNoopFlightClient.read(transport);
989 });
990
991 - it('should warn in DEV if a class instance is passed to a host component', () => {
991 + it('should error if a class instance is passed to a host component', () => {
992 class Foo {
993 method() {}
994 }
995 - expect(() => {
996 - const transport = ReactNoopFlightServer.render(
997 - <input value={new Foo()} />,
998 - );
999 - ReactNoopFlightClient.read(transport);
1000 - }).toErrorDev(
1001 - 'Only plain objects can be passed to Client Components from Server Components. ',
1002 - {withoutStack: true},
1003 - );
995 + const errors = [];
996 + ReactNoopFlightServer.render(<input value={new Foo()} />, {
997 + onError(x) {
998 + errors.push(x.message);
999 + },
1000 + });
1001 +
1002 + expect(errors).toEqual([
1003 + 'Only plain objects, and a few built-ins, can be passed to Client Components ' +
1004 + 'from Server Components. Classes or null prototypes are not supported.',
1005 + ]);
1006 });
1007
1008 it('should warn in DEV if a a client reference is passed to useContext()', () => {
packages/react-server/src/ReactFlightServer.js
+40 -26
@@ -111,10 +111,13 @@ import {getOrCreateServerContext} from 'shared/ReactServerContextRegistry';
111 import ReactSharedInternals from 'shared/ReactSharedInternals';
112 import ReactServerSharedInternals from './ReactServerSharedInternals';
113 import isArray from 'shared/isArray';
114 +import getPrototypeOf from 'shared/getPrototypeOf';
115 import binaryToComparableString from 'shared/binaryToComparableString';
116
117 import {SuspenseException, getSuspendedThenable} from './ReactFlightThenable';
118
119 +const ObjectPrototype = Object.prototype;
120 +
121 type JSONValue =
122 | string
123 | boolean
@@ -1046,6 +1049,11 @@ function resolveModelToJSON(
1049 return (undefined: any);
1050 }
1051
1052 + if (isArray(value)) {
1053 + // $FlowFixMe[incompatible-return]
1054 + return value;
1055 + }
1056 +
1057 if (value instanceof Map) {
1058 return serializeMap(request, value);
1059 }
@@ -1107,39 +1115,45 @@ function resolveModelToJSON(
1115 }
1116 }
1117
1110 - if (!isArray(value)) {
1111 - const iteratorFn = getIteratorFn(value);
1112 - if (iteratorFn) {
1113 - return Array.from((value: any));
1114 - }
1118 + const iteratorFn = getIteratorFn(value);
1119 + if (iteratorFn) {
1120 + return Array.from((value: any));
1121 }
1122
1123 + // Verify that this is a simple plain object.
1124 + const proto = getPrototypeOf(value);
1125 + if (
1126 + proto !== ObjectPrototype &&
1127 + (proto === null || getPrototypeOf(proto) !== null)
1128 + ) {
1129 + throw new Error(
1130 + 'Only plain objects, and a few built-ins, can be passed to Client Components ' +
1131 + 'from Server Components. Classes or null prototypes are not supported.',
1132 + );
1133 + }
1134 if (__DEV__) {
1118 - if (value !== null && !isArray(value)) {
1119 - // Verify that this is a simple plain object.
1120 - if (objectName(value) !== 'Object') {
1121 - console.error(
1122 - 'Only plain objects can be passed to Client Components from Server Components. ' +
1123 - '%s objects are not supported.%s',
1124 - objectName(value),
1125 - describeObjectForErrorMessage(parent, key),
1126 - );
1127 - } else if (!isSimpleObject(value)) {
1135 + if (objectName(value) !== 'Object') {
1136 + console.error(
1137 + 'Only plain objects can be passed to Client Components from Server Components. ' +
1138 + '%s objects are not supported.%s',
1139 + objectName(value),
1140 + describeObjectForErrorMessage(parent, key),
1141 + );
1142 + } else if (!isSimpleObject(value)) {
1143 + console.error(
1144 + 'Only plain objects can be passed to Client Components from Server Components. ' +
1145 + 'Classes or other objects with methods are not supported.%s',
1146 + describeObjectForErrorMessage(parent, key),
1147 + );
1148 + } else if (Object.getOwnPropertySymbols) {
1149 + const symbols = Object.getOwnPropertySymbols(value);
1150 + if (symbols.length > 0) {
1151 console.error(
1152 'Only plain objects can be passed to Client Components from Server Components. ' +
1130 - 'Classes or other objects with methods are not supported.%s',
1153 + 'Objects with symbol properties like %s are not supported.%s',
1154 + symbols[0].description,
1155 describeObjectForErrorMessage(parent, key),
1156 );
1133 - } else if (Object.getOwnPropertySymbols) {
1134 - const symbols = Object.getOwnPropertySymbols(value);
1135 - if (symbols.length > 0) {
1136 - console.error(
1137 - 'Only plain objects can be passed to Client Components from Server Components. ' +
1138 - 'Objects with symbol properties like %s are not supported.%s',
1139 - symbols[0].description,
1140 - describeObjectForErrorMessage(parent, key),
1141 - );
1142 - }
1157 }
1158 }
1159 }
packages/react/src/ReactTaint.js
+3 -3
@@ -9,6 +9,8 @@
9
10 import {enableTaint, enableBinaryFlight} from 'shared/ReactFeatureFlags';
11
12 +import getPrototypeOf from 'shared/getPrototypeOf';
13 +
14 import binaryToComparableString from 'shared/binaryToComparableString';
15
16 import ReactServerSharedInternals from './ReactServerSharedInternals';
@@ -22,9 +24,7 @@ const {
24 interface Reference {}
25
26 // This is the shared constructor of all typed arrays.
25 -const TypedArrayConstructor = Object.getPrototypeOf(
26 - Uint32Array.prototype,
27 -).constructor;
27 +const TypedArrayConstructor = getPrototypeOf(Uint32Array.prototype).constructor;
28
29 const defaultMessage =
30 'A tainted value was attempted to be serialized to a Client Component or Action closure. ' +
packages/shared/ReactSerializationErrors.js
+3 -2
@@ -19,6 +19,7 @@ import {
19 import type {LazyComponent} from 'react/src/ReactLazy';
20
21 import isArray from 'shared/isArray';
22 +import getPrototypeOf from 'shared/getPrototypeOf';
23
24 // Used for DEV messages to keep track of which parent rendered some props,
25 // in case they error.
@@ -35,7 +36,7 @@ function isObjectPrototype(object: any): boolean {
36 }
37 // It might be an object from a different Realm which is
38 // still just a plain simple object.
38 - if (Object.getPrototypeOf(object)) {
39 + if (getPrototypeOf(object)) {
40 return false;
41 }
42 const names = Object.getOwnPropertyNames(object);
@@ -48,7 +49,7 @@ function isObjectPrototype(object: any): boolean {
49 }
50
51 export function isSimpleObject(object: any): boolean {
51 - if (!isObjectPrototype(Object.getPrototypeOf(object))) {
52 + if (!isObjectPrototype(getPrototypeOf(object))) {
53 return false;
54 }
55 const names = Object.getOwnPropertyNames(object);
packages/shared/getPrototypeOf.js new
+12
@@ -0,0 +1,12 @@
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 +const getPrototypeOf = Object.getPrototypeOf;
11 +
12 +export default getPrototypeOf;
scripts/error-codes/codes.json
+3 -1
@@ -482,5 +482,7 @@
482 "494": "taintUniqueValue cannot taint objects or functions. Try taintObjectReference instead.",
483 "495": "Cannot taint a %s because the value is too general and not unique enough to block globally.",
484 "496": "Only objects or functions can be passed to taintObjectReference. Try taintUniqueValue instead.",
485 - "497": "Only objects or functions can be passed to taintObjectReference."
485 + "497": "Only objects or functions can be passed to taintObjectReference.",
486 + "498": "Only plain objects, and a few built-ins, can be passed to Client Components from Server Components. Classes or null prototypes are not supported.",
487 + "499": "Only plain objects, and a few built-ins, can be passed to Server Actions. Classes or null prototypes are not supported."
488 }
\ No newline at end of file