Impossible to avoid using a nonce without XSS vulnerability #5162
Comments
|
I'm live streaming this now, so feel free to watch me demo the issue: https://www.youtube.com/watch?v=YboFmtwxIBk&t=2h42m40s |
|
@kentcdodds I think it's currently possible to use a nonce without exposing it to the client. I created a sample repo showing that. Here is the actual commit where I added it to the plain remix project. The idea is to use React context with a value set only on the server. To do that we will need:
// app/components/nonce-context.tsx
import React from "react";
// The default value (`undefined`) will be used on the client
export const NonceContext = React.createContext<string | undefined>(undefined);
// app/root.tsx
// ...
import { useContext } from "react";
import { NonceContext } from "./components/nonce-context";
// ...
export default function App() {
const nonce = useContext(NonceContext);
return (
<html lang="en">
<head>
<Meta />
<Links />
</head>
<body>
<Outlet />
<ScrollRestoration nonce={nonce} />
<Scripts nonce={nonce} />
<LiveReload nonce={nonce} />
{/* Uncomment the `<script>` below to ensure the CSP policy prevented
an inline script without nonce
*/}
{/* <script
dangerouslySetInnerHTML={{ __html: "alert('Should be prevented')" }}
/> */}
</body>
</html>
);
}
// app/entry.server.tsx
import crypto from "node:crypto";
// ...
import { NonceContext } from "./components/nonce-context";
// ...
function handleBrowserRequest(
request: Request,
responseStatusCode: number,
responseHeaders: Headers,
remixContext: EntryContext
) {
const cspNonce = crypto.randomBytes(16).toString("hex");
responseHeaders.set(
"Content-Security-Policy",
`script-src 'nonce-${cspNonce}' 'strict-dynamic'; object-src 'none'; base-uri 'none';`
);
return new Promise((resolve, reject) => {
// ...
const { pipe, abort } = renderToPipeableStream(
// Set value for the provider to use it on the server
<NonceContext.Provider value={cspNonce}>
<RemixServer context={remixContext} url={request.url} />
</NonceContext.Provider>,
{
// Also set nonce for Suspense
nonce: cspNonce,
// ...
}
);
// ...
});
} |
|
Oh, that's brilliant and almost so obvious I'm embarrassed I didn't think of it 🤦♂️. Thanks for the tip! I'm going to do that now. Might be room to stick this in the docs somewhere or make an example. |
|
Dang it. Unfortunately this only really works well if you're generating the nonce within the |
|
We need this: Until then, I've shipped a patch-package of Remix to side-step this security issue. |
|
@kentcdodds Can you expand on the risk of having the nonce in the SSR injected block for In your video you mentioned that the console.log([...document.getElementsByTagName("script")].find(x => x.nonce).nonce);Evidently this is part of the w3c spec and an issue is documented at w3c/webappsec-csp#458 |
|
It's because non-blessed js (like an extension) can access the DOM and therefore can grab the nonce from the script that sets the Remix context variable. |
|
If the script is "blessed" then the nonce doesn't matter anyway. |
|
I'm not seeing how the patching to remove the data changes the threat risk. If it does, then how? I don't think extensions are included in the threat model of the Content-Security-Policy spec. When a script tag has a matching nonce, its origin domain is on the list, or has its hash is included in the Content-Security-Policy header, then that is what I meant as "blessed" by the developer. If any script The extensions' Manifest V3 allows Manifest V2 allowed injecting script blocks and and combined with extensions' ability to modify the page's CSP headers, I think any script can be injected no matter what a site owner does. Setting site CSP headers isn't going to stop that. See the uBlock extension code for an example that adds an additional header, but it could have rewrote an existing header. Unfortunately, the network tab in the DevTools does not show the resulting modified headers. I added some logging to a sample extension to view the resulting headers. https://gist.github.com/ngbrown/e67894d87e5298556afae180c591afef |
|
It sounds like you're suggesting that nonce doesn't actually protect you in any way 🤔 can you demonstrate this by making an extension that exploits this on a website that utilizes a nonce? |
|
Maybe it just doesn't protect a website in the way you thought it did? The issue I pointed to in the w3c CSP spec repo included links to this talk and slides. Maybe that would help us understand the actual benefits and reason behind CSP? I had a Manifest v3 extension that demonstrated what I was saying in the gist in my last post. Save the files, turn on developer mode in your extensions and load it as an unpacked extension. It extracts the nonce from a Or did you want a demonstration of a script element injection in a Manifest V2 extension? |
|
Great stuff in this thread! I'm almost happy with the solution however the potentially Arc? code I can't seem to find and configure. Does this ring a bell to anyone? This is the last remaining thing that needs a nonce (and is only an inconvenience on a dev server) <script>
(() => {
const url = 'ws://localhost:2222'
let socket = new WebSocket(url)
socket.addEventListener('message', message => {
if (message.data === 'reload') location.reload()
})
socket.addEventListener('close', ({ wasClean }) => {
const retryMs = 1000
const cancelMs = 5000
const maxAttempts = Math.round(cancelMs / retryMs)
let attempts = 0
const reloadIfCanConnect = () => {
attempts++
if (attempts > maxAttempts){
console.error('Could not reconnect to dev server.')
return
}
socket = new WebSocket(url)
socket.addEventListener('error', () => {
setTimeout(reloadIfCanConnect, retryMs)
})
socket.addEventListener('open', () => {
if (!wasClean) {
location.reload()
}
})
}
reloadIfCanConnect()
})
})();
</script> |
|
This code comes from the LiveReload component. You need to supply a nonce prop to that. It's marked as deprecated, but it's not (#5161). |
|
I've got this but the error's still there... maybe I need a clean build?
|
|
Hmmm... Maybe open a bug report with a reproduction? |
|
Ok, the plot chickens. There is an actual Live Reload code at the bottom of the BODY and it does indeed have the nonce. |
|
Found it, it's Arc's livereload. Do I need both? I don't think so, right?
|
|
I don't know Arc 🤷♂️ Probably not? |
|
Have you had any luck suppressing the hydration warnings due to the browser removing the nonce value from the html? |
|
You shouldn't hydrate with a nonce. The client shouldn't have access to the nonce value. |
|
I'm struggling to figure out how to do that. My scripts are only loaded through the root.tsx and nowhere else. Where would I need to put them? (They're also scripts with innerhtml, not remote ones) |
|
Do it like this: #5162 (comment) |
|
Yeah, this is exactly how I am doing it. Was following this from the beginning. Still getting the error hence my confusion. Thanks for helping though, I really appreciate it. https://github.com/meza/trance-stack/blob/14aaffb726183ba8c784a7e4d6e2ae0a603f0264/src/root.tsx#L88 For now we opted to do the same thing you do with LiveReload and the ScrollRestoration and suppress the warnings but it does leave a bitter taste in my mouth. |
|
I'm getting hydration errors with this approach since v1.16.0 of remix. Created a new issue for it: #6394 |
|
Have put together an alternative solution based on Node's |
|
@kentcdodds What's the latest solution here? |
|
Checkout what the Epic Stack does: https://github.com/epicweb-dev/epic-stack |
What version of Remix are you using?
1.11.0
Steps to Reproduce
I've created a demo: https://github.com/kentcdodds/nonce-remix-issue
Basically, create a project that uses
nonceCSP and try to ensure thenonceis both available for the server render but not available for the client hydration.Expected Behavior
I'd like a mechanism that allows me to mark specific data as not able to be passed to the client.
Actual Behavior
The only way to get data into the UI on the server is through the loader. Everything the loader sends to UI on the server also goes to the client. Therefore, the nonce we need for the server UI makes it into the client, which basically makes us lose all benefits of a nonce CSP.
The text was updated successfully, but these errors were encountered: