Skip to content

Commit

Permalink
[Flight Reply] Don't allow Symbols to be passed to a reply (#28610)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sebmarkbage committed Mar 21, 2024
1 parent c47fee5 commit 72e02a8
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 23 deletions.
20 changes: 7 additions & 13 deletions packages/react-client/src/ReactFlightReplyClient.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,6 @@ function serializeTemporaryReferenceID(id: number): string {
return '$T' + id.toString(16);
}

function serializeSymbolReference(name: string): string {
return '$S' + name;
}

function serializeFormDataReference(id: number): string {
// Why K? F is "Function". D is "Date". What else?
return '$K' + id.toString(16);
Expand Down Expand Up @@ -479,18 +475,16 @@ export function processReply(
}

if (typeof value === 'symbol') {
// $FlowFixMe[incompatible-type] `description` might be undefined
const name: string = value.description;
if (Symbol.for(name) !== value) {
if (temporaryReferences === undefined) {
throw new Error(
'Only global symbols received from Symbol.for(...) can be passed to Server Functions. ' +
`The symbol Symbol.for(${
// $FlowFixMe[incompatible-type] `description` might be undefined
value.description
}) cannot be found among global symbols.`,
'Symbols cannot be passed to a Server Function without a ' +
'temporary reference set. Pass a TemporaryReferenceSet to the options.' +
(__DEV__ ? describeObjectForErrorMessage(parent, key) : ''),
);
}
return serializeSymbolReference(name);
return serializeTemporaryReferenceID(
writeTemporaryReference(temporaryReferences, value),
);
}

if (typeof value === 'bigint') {
Expand Down
10 changes: 5 additions & 5 deletions packages/react-client/src/ReactFlightTemporaryReferences.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,15 @@

interface Reference {}

export opaque type TemporaryReferenceSet = Array<Reference>;
export opaque type TemporaryReferenceSet = Array<Reference | symbol>;

export function createTemporaryReferenceSet(): TemporaryReferenceSet {
return [];
}

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

export function readTemporaryReference(
export function readTemporaryReference<T>(
set: TemporaryReferenceSet,
id: number,
): Reference {
): T {
if (id < 0 || id >= set.length) {
throw new Error(
"The RSC response contained a reference that doesn't exist in the temporary reference set. " +
'Always pass the matching set that was used to create the reply when parsing its response.',
);
}
return set[id];
return (set[id]: any);
}
4 changes: 0 additions & 4 deletions packages/react-server/src/ReactFlightReplyServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -396,10 +396,6 @@ function parseModelString(
const chunk = getChunk(response, id);
return chunk;
}
case 'S': {
// Symbol
return Symbol.for(value.slice(2));
}
case 'F': {
// Server Reference
const id = parseInt(value.slice(2), 16);
Expand Down
3 changes: 2 additions & 1 deletion scripts/error-codes/codes.json
Original file line number Diff line number Diff line change
Expand Up @@ -501,5 +501,6 @@
"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.",
"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.",
"515": "Cannot assign to a temporary client reference from a server module.",
"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."
"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.",
"517": "Symbols cannot be passed to a Server Function without a temporary reference set. Pass a TemporaryReferenceSet to the options.%s"
}

0 comments on commit 72e02a8

Please sign in to comment.