Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Handle nonce hydration warnings better #26028

Open
kentcdodds opened this issue Jan 20, 2023 · 4 comments
Open

Bug: Handle nonce hydration warnings better #26028

kentcdodds opened this issue Jan 20, 2023 · 4 comments
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug

Comments

@kentcdodds
Copy link
Contributor

React version: 18.2.0

Steps To Reproduce

  1. Create an app with a CSP
  2. Server render your app with a nonce in script tags and hydrate those tags with or without the nonce prop and observe warnings that insufficiently describe the seriousness of security vulnerabilities and how to fix them.

Link to code example:

https://github.com/kentcdodds/nonce-hydration-issues

The current behavior

We get warnings about extra nonce prop or a mismatch of the nonce value which are insufficient.

The expected behavior

Most of the issue is described in the README for the project. But we've got two warnings that could be improved. In the case of the client hydrating without the nonce (because it correctly doesn't know what the nonce value is), the warning should be either removed or improved to explain you should pass an empty string for the nonce value to match what the browser did to it.

In the case of the client hydrating with the nonce (because it incorrectly does know what the nonce value is), the warning should explain that they have a security vulnerability that should be fixed because if the client hydration code knows what the nonce value is then XSS attackers could know as well.

Suggested solutions:

(copied from the project readme):

Update the error messages to explain the problem and solution.

  • The Extra attributes from the server: nonce error message should be updated
    to explain issue with nonce. Maybe something like:
    Extra attributes from the server: nonce. The nonce attribute is a security feature and should not be passed to the client. Instead, set the nonce attribute to an empty string when client rendering.
    Alternatively, you could just not warn about the extra nonce attribute at
    all.
  • The Prop nonce did not match error message should be updated to explain the
    issue with nonce. Maybe something like:
    Prop nonce did not match. The nonce attribute is a security feature and should not be passed to the client as yours appears to do currently which makes your application vulnerable to cross-site scripting attacks. Instead, set the nonce attribute to an empty string when client rendering. Make certain to not send the nonce value in the DOM anywhere other than in the nonce attribute of scripts and links.
    Whatever we do, we definitely want to make sure people are aware this is a
    serious security vulnerability.

Video walkthrough:

I'm live streaming this now. Here's my walkthrough of the problem: https://www.youtube.com/watch?v=YboFmtwxIBk&t=1h57m

@kentcdodds kentcdodds added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Jan 20, 2023
@YounessFkhach
Copy link

YounessFkhach commented Jan 20, 2023

what you are missing out is that while you can't access the nonce from element.getAttribute("nonce")
you cann access it from inside the script by calling script.nonce

the browser doesn't clear it, but just prevents reading it from outside of the script itself

while I agree that if there was no nonce from the server, we shouldn't have to pass an empty string

P.S read the discord chat

@kentcdodds
Copy link
Contributor Author

Even if you can access the nonce from the JS code, you shouldn't render the actual nonce value anywhere in the DOM because as soon as you do it is going to open you up to XSS vulnerabilities.

@sophiebits
Copy link
Collaborator

@jamischarles
Copy link
Contributor

jamischarles commented Feb 28, 2024

Alternatively, you could just not warn about the extra nonce attribute at
all

Any easy way / workarounds to suppress this error? I'm seeing this error when I render a vanilla <script> tag in an RSC (via next.js app router)

Adding suppressHydrationWarning to my script tag doesn't seem to work either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug
Projects
None yet
Development

No branches or pull requests

4 participants