react icon indicating copy to clipboard operation
react copied to clipboard

Bug: Handle nonce hydration warnings better

Open kentcdodds opened this issue 2 years ago • 4 comments

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 avatar Jan 20 '23 18:01 kentcdodds

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

YounessFkhach avatar Jan 20 '23 18:01 YounessFkhach

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.

kentcdodds avatar Jan 20 '23 19:01 kentcdodds

Related spec discussion:

  • https://github.com/whatwg/html/pull/2373

sophiebits avatar Feb 25 '23 00:02 sophiebits

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.

jamischarles avatar Feb 28 '24 18:02 jamischarles

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

github-actions[bot] avatar May 28 '24 19:05 github-actions[bot]

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you!

github-actions[bot] avatar Jun 04 '24 20:06 github-actions[bot]

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.

Using Script from next/script instead of the vanilla script would fix it.

Guo-dalu avatar Jun 20 '24 22:06 Guo-dalu