[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.
facebook · Oct 11, 2023 · e61a60f · e61a60f
1 parent 1fc5828
commit e61a60f
Show file tree

Hide file tree

Showing 7 changed files with 127 additions and 83 deletions.
diff --git a/packages/react-client/src/ReactFlightReplyClient.js b/packages/react-client/src/ReactFlightReplyClient.js
@@ -29,6 +29,9 @@ import {
 } from 'shared/ReactSerializationErrors';
 
 import isArray from 'shared/isArray';
+import getPrototypeOf from 'shared/getPrototypeOf';
+
+const ObjectPrototype = Object.prototype;
 
 import {usedWithSSR} from './ReactFlightClientConfig';
 
@@ -227,6 +230,10 @@ export function processReply(
         );
         return serializePromiseID(promiseId);
       }
+      if (isArray(value)) {
+        // $FlowFixMe[incompatible-return]
+        return value;
+      }
       // TODO: Should we the Object.prototype.toString.call() to test for cross-realm objects?
       if (value instanceof FormData) {
         if (formData === null) {
@@ -263,54 +270,60 @@ export function processReply(
         formData.append(formFieldPrefix + setId, partJSON);
         return serializeSetID(setId);
       }
-      if (!isArray(value)) {
-        const iteratorFn = getIteratorFn(value);
-        if (iteratorFn) {
-          return Array.from((value: any));
-        }
+      const iteratorFn = getIteratorFn(value);
+      if (iteratorFn) {
+        return Array.from((value: any));
       }
 
+      // Verify that this is a simple plain object.
+      const proto = getPrototypeOf(value);
+      if (
+        proto !== ObjectPrototype &&
+        (proto === null || getPrototypeOf(proto) !== null)
+      ) {
+        throw new Error(
+          'Only plain objects, and a few built-ins, can be passed to Server Actions. ' +
+            'Classes or null prototypes are not supported.',
+        );
+      }
       if (__DEV__) {
-        if (value !== null && !isArray(value)) {
-          // Verify that this is a simple plain object.
-          if ((value: any).$$typeof === REACT_ELEMENT_TYPE) {
-            console.error(
-              'React Element cannot be passed to Server Functions from the Client.%s',
-              describeObjectForErrorMessage(parent, key),
-            );
-          } else if ((value: any).$$typeof === REACT_LAZY_TYPE) {
-            console.error(
-              'React Lazy cannot be passed to Server Functions from the Client.%s',
-              describeObjectForErrorMessage(parent, key),
-            );
-          } else if ((value: any).$$typeof === REACT_PROVIDER_TYPE) {
-            console.error(
-              'React Context Providers cannot be passed to Server Functions from the Client.%s',
-              describeObjectForErrorMessage(parent, key),
-            );
-          } else if (objectName(value) !== 'Object') {
-            console.error(
-              'Only plain objects can be passed to Client Components from Server Components. ' +
-                '%s objects are not supported.%s',
-              objectName(value),
-              describeObjectForErrorMessage(parent, key),
-            );
-          } else if (!isSimpleObject(value)) {
+        if ((value: any).$$typeof === REACT_ELEMENT_TYPE) {
+          console.error(
+            'React Element cannot be passed to Server Functions from the Client.%s',
+            describeObjectForErrorMessage(parent, key),
+          );
+        } else if ((value: any).$$typeof === REACT_LAZY_TYPE) {
+          console.error(
+            'React Lazy cannot be passed to Server Functions from the Client.%s',
+            describeObjectForErrorMessage(parent, key),
+          );
+        } else if ((value: any).$$typeof === REACT_PROVIDER_TYPE) {
+          console.error(
+            'React Context Providers cannot be passed to Server Functions from the Client.%s',
+            describeObjectForErrorMessage(parent, key),
+          );
+        } else if (objectName(value) !== 'Object') {
+          console.error(
+            'Only plain objects can be passed to Client Components from Server Components. ' +
+              '%s objects are not supported.%s',
+            objectName(value),
+            describeObjectForErrorMessage(parent, key),
+          );
+        } else if (!isSimpleObject(value)) {
+          console.error(
+            'Only plain objects can be passed to Client Components from Server Components. ' +
+              'Classes or other objects with methods are not supported.%s',
+            describeObjectForErrorMessage(parent, key),
+          );
+        } else if (Object.getOwnPropertySymbols) {
+          const symbols = Object.getOwnPropertySymbols(value);
+          if (symbols.length > 0) {
             console.error(
               'Only plain objects can be passed to Client Components from Server Components. ' +
-                'Classes or other objects with methods are not supported.%s',
+                'Objects with symbol properties like %s are not supported.%s',
+              symbols[0].description,
               describeObjectForErrorMessage(parent, key),
             );
-          } else if (Object.getOwnPropertySymbols) {
-            const symbols = Object.getOwnPropertySymbols(value);
-            if (symbols.length > 0) {
-              console.error(
-                'Only plain objects can be passed to Client Components from Server Components. ' +
-                  'Objects with symbol properties like %s are not supported.%s',
-                symbols[0].description,
-                describeObjectForErrorMessage(parent, key),
-              );
-            }
           }
         }
       }

diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js
@@ -988,19 +988,21 @@ describe('ReactFlight', () => {
     ReactNoopFlightClient.read(transport);
   });
 
-  it('should warn in DEV if a class instance is passed to a host component', () => {
+  it('should error if a class instance is passed to a host component', () => {
     class Foo {
       method() {}
     }
-    expect(() => {
-      const transport = ReactNoopFlightServer.render(
-        <input value={new Foo()} />,
-      );
-      ReactNoopFlightClient.read(transport);
-    }).toErrorDev(
-      'Only plain objects can be passed to Client Components from Server Components. ',
-      {withoutStack: true},
-    );
+    const errors = [];
+    ReactNoopFlightServer.render(<input value={new Foo()} />, {
+      onError(x) {
+        errors.push(x.message);
+      },
+    });
+
+    expect(errors).toEqual([
+      'Only plain objects, and a few built-ins, can be passed to Client Components ' +
+        'from Server Components. Classes or null prototypes are not supported.',
+    ]);
   });
 
   it('should warn in DEV if a a client reference is passed to useContext()', () => {

diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js
@@ -111,10 +111,13 @@ import {getOrCreateServerContext} from 'shared/ReactServerContextRegistry';
 import ReactSharedInternals from 'shared/ReactSharedInternals';
 import ReactServerSharedInternals from './ReactServerSharedInternals';
 import isArray from 'shared/isArray';
+import getPrototypeOf from 'shared/getPrototypeOf';
 import binaryToComparableString from 'shared/binaryToComparableString';
 
 import {SuspenseException, getSuspendedThenable} from './ReactFlightThenable';
 
+const ObjectPrototype = Object.prototype;
+
 type JSONValue =
   | string
   | boolean
@@ -1046,6 +1049,11 @@ function resolveModelToJSON(
       return (undefined: any);
     }
 
+    if (isArray(value)) {
+      // $FlowFixMe[incompatible-return]
+      return value;
+    }
+
     if (value instanceof Map) {
       return serializeMap(request, value);
     }
@@ -1107,39 +1115,45 @@ function resolveModelToJSON(
       }
     }
 
-    if (!isArray(value)) {
-      const iteratorFn = getIteratorFn(value);
-      if (iteratorFn) {
-        return Array.from((value: any));
-      }
+    const iteratorFn = getIteratorFn(value);
+    if (iteratorFn) {
+      return Array.from((value: any));
     }
 
+    // Verify that this is a simple plain object.
+    const proto = getPrototypeOf(value);
+    if (
+      proto !== ObjectPrototype &&
+      (proto === null || getPrototypeOf(proto) !== null)
+    ) {
+      throw new Error(
+        'Only plain objects, and a few built-ins, can be passed to Client Components ' +
+          'from Server Components. Classes or null prototypes are not supported.',
+      );
+    }
     if (__DEV__) {
-      if (value !== null && !isArray(value)) {
-        // Verify that this is a simple plain object.
-        if (objectName(value) !== 'Object') {
-          console.error(
-            'Only plain objects can be passed to Client Components from Server Components. ' +
-              '%s objects are not supported.%s',
-            objectName(value),
-            describeObjectForErrorMessage(parent, key),
-          );
-        } else if (!isSimpleObject(value)) {
+      if (objectName(value) !== 'Object') {
+        console.error(
+          'Only plain objects can be passed to Client Components from Server Components. ' +
+            '%s objects are not supported.%s',
+          objectName(value),
+          describeObjectForErrorMessage(parent, key),
+        );
+      } else if (!isSimpleObject(value)) {
+        console.error(
+          'Only plain objects can be passed to Client Components from Server Components. ' +
+            'Classes or other objects with methods are not supported.%s',
+          describeObjectForErrorMessage(parent, key),
+        );
+      } else if (Object.getOwnPropertySymbols) {
+        const symbols = Object.getOwnPropertySymbols(value);
+        if (symbols.length > 0) {
           console.error(
             'Only plain objects can be passed to Client Components from Server Components. ' +
-              'Classes or other objects with methods are not supported.%s',
+              'Objects with symbol properties like %s are not supported.%s',
+            symbols[0].description,
             describeObjectForErrorMessage(parent, key),
           );
-        } else if (Object.getOwnPropertySymbols) {
-          const symbols = Object.getOwnPropertySymbols(value);
-          if (symbols.length > 0) {
-            console.error(
-              'Only plain objects can be passed to Client Components from Server Components. ' +
-                'Objects with symbol properties like %s are not supported.%s',
-              symbols[0].description,
-              describeObjectForErrorMessage(parent, key),
-            );
-          }
         }
       }
     }

diff --git a/packages/react/src/ReactTaint.js b/packages/react/src/ReactTaint.js
@@ -9,6 +9,8 @@
 
 import {enableTaint, enableBinaryFlight} from 'shared/ReactFeatureFlags';
 
+import getPrototypeOf from 'shared/getPrototypeOf';
+
 import binaryToComparableString from 'shared/binaryToComparableString';
 
 import ReactServerSharedInternals from './ReactServerSharedInternals';
@@ -22,9 +24,7 @@ const {
 interface Reference {}
 
 // This is the shared constructor of all typed arrays.
-const TypedArrayConstructor = Object.getPrototypeOf(
-  Uint32Array.prototype,
-).constructor;
+const TypedArrayConstructor = getPrototypeOf(Uint32Array.prototype).constructor;
 
 const defaultMessage =
   'A tainted value was attempted to be serialized to a Client Component or Action closure. ' +

diff --git a/packages/shared/ReactSerializationErrors.js b/packages/shared/ReactSerializationErrors.js
@@ -19,6 +19,7 @@ import {
 import type {LazyComponent} from 'react/src/ReactLazy';
 
 import isArray from 'shared/isArray';
+import getPrototypeOf from 'shared/getPrototypeOf';
 
 // Used for DEV messages to keep track of which parent rendered some props,
 // in case they error.
@@ -35,7 +36,7 @@ function isObjectPrototype(object: any): boolean {
   }
   // It might be an object from a different Realm which is
   // still just a plain simple object.
-  if (Object.getPrototypeOf(object)) {
+  if (getPrototypeOf(object)) {
     return false;
   }
   const names = Object.getOwnPropertyNames(object);
@@ -48,7 +49,7 @@ function isObjectPrototype(object: any): boolean {
 }
 
 export function isSimpleObject(object: any): boolean {
-  if (!isObjectPrototype(Object.getPrototypeOf(object))) {
+  if (!isObjectPrototype(getPrototypeOf(object))) {
     return false;
   }
   const names = Object.getOwnPropertyNames(object);

diff --git a/packages/shared/getPrototypeOf.js b/packages/shared/getPrototypeOf.js
@@ -0,0 +1,12 @@
+/**
+ * Copyright (c) Meta Platforms, Inc. and affiliates.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ *
+ * @flow
+ */
+
+const getPrototypeOf = Object.getPrototypeOf;
+
+export default getPrototypeOf;
diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json
@@ -482,5 +482,7 @@
   "494": "taintUniqueValue cannot taint objects or functions. Try taintObjectReference instead.",
   "495": "Cannot taint a %s because the value is too general and not unique enough to block globally.",
   "496": "Only objects or functions can be passed to taintObjectReference. Try taintUniqueValue instead.",
-  "497": "Only objects or functions can be passed to taintObjectReference."
+  "497": "Only objects or functions can be passed to taintObjectReference.",
+  "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.",
+  "499": "Only plain objects, and a few built-ins, can be passed to Server Actions. Classes or null prototypes are not supported."
 }