Skip to content

Commit 72e02a8

Browse files
authored
[Flight Reply] Don't allow Symbols to be passed to a reply (#28610)
As mentioned in #28609 there's a potential security risk if you allow a passed value to the server to spoof Elements because it allows a hacker to POST cross origin. This is only an issue if your framework allows this which it shouldn't but it seems like we should provide an extra layer of security here. ```js function action(errors, payload) { try { ... } catch (x) { return [newError].concat(errors); } } ``` ```js const [errors, formAction] = useActionState(action); return <div>{errors}</div>; ``` This would allow you to construct a payload where the previous "errors" set includes something like `<script src="danger.js" />`. We could block only elements from being received but it could potentially be a risk with creating other React types like Context too. We use symbols as a way to securely brand these. Most JS don't use this kind of branding with symbols like we do. They're generally properties which we don't support anyway. However in theory someone else could be using them like we do. So in an abundance of carefulness I just ban all symbols from being passed (except by temporary reference) - not just ours. This means that the format isn't fully symmetric even beyond just React Nodes. #28611 allows code that includes symbols/elements to continue working but may have to bail out to replaying instead of no JS sometimes. However, you still can't access the symbols inside the server - they're by reference only.
1 parent c47fee5 commit 72e02a8

File tree

4 files changed

+14
-23
lines changed

4 files changed

+14
-23
lines changed

packages/react-client/src/ReactFlightReplyClient.js

+7-13
Original file line numberDiff line numberDiff line change
@@ -105,10 +105,6 @@ function serializeTemporaryReferenceID(id: number): string {
105105
return '$T' + id.toString(16);
106106
}
107107

108-
function serializeSymbolReference(name: string): string {
109-
return '$S' + name;
110-
}
111-
112108
function serializeFormDataReference(id: number): string {
113109
// Why K? F is "Function". D is "Date". What else?
114110
return '$K' + id.toString(16);
@@ -479,18 +475,16 @@ export function processReply(
479475
}
480476

481477
if (typeof value === 'symbol') {
482-
// $FlowFixMe[incompatible-type] `description` might be undefined
483-
const name: string = value.description;
484-
if (Symbol.for(name) !== value) {
478+
if (temporaryReferences === undefined) {
485479
throw new Error(
486-
'Only global symbols received from Symbol.for(...) can be passed to Server Functions. ' +
487-
`The symbol Symbol.for(${
488-
// $FlowFixMe[incompatible-type] `description` might be undefined
489-
value.description
490-
}) cannot be found among global symbols.`,
480+
'Symbols cannot be passed to a Server Function without a ' +
481+
'temporary reference set. Pass a TemporaryReferenceSet to the options.' +
482+
(__DEV__ ? describeObjectForErrorMessage(parent, key) : ''),
491483
);
492484
}
493-
return serializeSymbolReference(name);
485+
return serializeTemporaryReferenceID(
486+
writeTemporaryReference(temporaryReferences, value),
487+
);
494488
}
495489

496490
if (typeof value === 'bigint') {

packages/react-client/src/ReactFlightTemporaryReferences.js

+5-5
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,15 @@
99

1010
interface Reference {}
1111

12-
export opaque type TemporaryReferenceSet = Array<Reference>;
12+
export opaque type TemporaryReferenceSet = Array<Reference | symbol>;
1313

1414
export function createTemporaryReferenceSet(): TemporaryReferenceSet {
1515
return [];
1616
}
1717

1818
export function writeTemporaryReference(
1919
set: TemporaryReferenceSet,
20-
object: Reference,
20+
object: Reference | symbol,
2121
): number {
2222
// We always create a new entry regardless if we've already written the same
2323
// object. This ensures that we always generate a deterministic encoding of
@@ -27,15 +27,15 @@ export function writeTemporaryReference(
2727
return newId;
2828
}
2929

30-
export function readTemporaryReference(
30+
export function readTemporaryReference<T>(
3131
set: TemporaryReferenceSet,
3232
id: number,
33-
): Reference {
33+
): T {
3434
if (id < 0 || id >= set.length) {
3535
throw new Error(
3636
"The RSC response contained a reference that doesn't exist in the temporary reference set. " +
3737
'Always pass the matching set that was used to create the reply when parsing its response.',
3838
);
3939
}
40-
return set[id];
40+
return (set[id]: any);
4141
}

packages/react-server/src/ReactFlightReplyServer.js

-4
Original file line numberDiff line numberDiff line change
@@ -396,10 +396,6 @@ function parseModelString(
396396
const chunk = getChunk(response, id);
397397
return chunk;
398398
}
399-
case 'S': {
400-
// Symbol
401-
return Symbol.for(value.slice(2));
402-
}
403399
case 'F': {
404400
// Server Reference
405401
const id = parseInt(value.slice(2), 16);

scripts/error-codes/codes.json

+2-1
Original file line numberDiff line numberDiff line change
@@ -501,5 +501,6 @@
501501
"513": "Cannot render a Client Context Provider on the Server. Instead, you can export a Client Component wrapper that itself renders a Client Context Provider.",
502502
"514": "Cannot access %s on the server. You cannot dot into a temporary client reference from a server component. You can only pass the value through to the client.",
503503
"515": "Cannot assign to a temporary client reference from a server module.",
504-
"516": "Attempted to call a temporary Client Reference from the server but it is on the client. It's not possible to invoke a client function from the server, it can only be rendered as a Component or passed to props of a Client Component."
504+
"516": "Attempted to call a temporary Client Reference from the server but it is on the client. It's not possible to invoke a client function from the server, it can only be rendered as a Component or passed to props of a Client Component.",
505+
"517": "Symbols cannot be passed to a Server Function without a temporary reference set. Pass a TemporaryReferenceSet to the options.%s"
505506
}

0 commit comments

Comments
 (0)