Docs: CSP documentation and examples render users vulnerable to headers injection

[viters]

What is the improvement or update you wish to see?

Official documentation and official examples contain code that is putting the applications using it in considerable danger. It is further escalated due to this code being copied and propagated: google search.

Vulnerable example: https://github.com/vercel/next.js/tree/canary/examples/with-strict-csp

Code snippets and examples should not contain a code that makes the applications using it vulnerable. I have reported this to responsible.disclosure@vercel.com, but was redirected here.

I do not see a reason to set nonce and CSP headers on request headers, so the snippet should just be modifying response headers, without touching request at all.

Is there any context that might help us understand?

const requestHeaders = new Headers(request.headers)
// [...]
return NextResponse.next({
  headers: requestHeaders,
  request: {
    headers: requestHeaders,
  },
})

This code is copying all request headers into server response. It allows the actor to inject malicious headers that will be returned by the server. It also allows modification of other headers, like Cache-Control, which might ex. result in caching malicious responses on CDN.

Proof of concept

1. Download and run the example: https://github.com/vercel/next.js/tree/canary/examples/with-strict-csp
2. Run curl -v -o /dev/null -s -H 'Cache-Control: malicious' -H 'X-Malicious: Anyvalue' http://localhost:3000/

*   Trying 127.0.0.1:3000...
* Connected to localhost (127.0.0.1) port 3000 (#0)
> GET / HTTP/1.1
> Host: localhost:3000
> User-Agent: curl/7.86.0
> Accept: */*
> Cache-Control: malicious
> X-Malicious: Anyvalue
>
* Mark bundle as not supporting multiuse
< HTTP/1.1 200 OK
< accept: */*
< cache-control: malicious
< content-security-policy: default-src 'self'; script-src 'self' 'nonce-MTJmNTdhOTItYTRkMi00Njk3LWJhZWYtYzM3OTRhMjcwZmJh' 'strict-dynamic'; style-src 'self' 'nonce-MTJmNTdhOTItYTRkMi00Njk3LWJhZWYtYzM3OTRhMjcwZmJh'; img-src 'self' blob: data:; font-src 'self'; object-src 'none'; base-uri 'self'; form-action 'self'; frame-ancestors 'none'; block-all-mixed-content; upgrade-insecure-requests;
< host: localhost:3000
< user-agent: curl/7.86.0
< x-malicious: Anyvalue
< x-nonce: MTJmNTdhOTItYTRkMi00Njk3LWJhZWYtYzM3OTRhMjcwZmJh
< Vary: RSC, Next-Router-State-Tree, Next-Router-Prefetch, Next-Url, Accept-Encoding
< X-Powered-By: Next.js
< Content-Type: text/html; charset=utf-8
< Date: Tue, 24 Oct 2023 20:05:11 GMT
< Connection: keep-alive
< Keep-Alive: timeout=5
< Transfer-Encoding: chunked
<

Does the docs page already exist? Please link to it.

https://nextjs.org/docs/pages/building-your-application/configuring/content-security-policy

[karlhorky]

const requestHeaders = new Headers(request.headers)
// [...]
return NextResponse.next({
  headers: requestHeaders,
  request: {
    headers: requestHeaders,
  },
})

This code is copying all request headers into server response. It allows the actor to inject malicious headers that will be returned by the server. It also allows modification of other headers, like Cache-Control, which might ex. result in caching malicious responses on CDN.

Thanks for this, sounds bad! 👀

cc @danieltott @gnoff

Can you offer a code example that would resolve the problem while still retaining the functionality of adding CSP (and optionally, a nonce, eg. x-nonce)?

[karlhorky]

Further context: copying the Content-Security-Policy to the request is required for React to read the header there:

// Clone the request headers
const requestHeaders = new Headers(request.headers);

// Set the Content-Security-Policy header in the request for
// App Router routes - this request header will be read in the
// `renderToHTMLOrFlight` function in Next.js and:
//
// 1. The header will be parsed to extract the nonce
// 2. The nonce will be added to the HTML tags which are
//    generated by Next.js (eg. the <script /> tags for the
//    framework)
//
// This is required in addition to the CSP response header below
//
// Ref: https://github.com/vercel/next.js/blob/29c2e89bd11b644d6665652d3b5c0314fdb0cdc5/packages/next/src/server/app-render/app-render.tsx#L1222-L1227
// Ref: https://github.com/vercel/next.js/issues/43743#issuecomment-1542712188
requestHeaders.set('Content-Security-Policy', cspHeader);

[karlhorky]

cc @leerob @ijjk I think the part in the docs cloning the request headers to the response headers was introduced in the PR created + reviewed by you:

• https://github.com/vercel/next.js/pull/54601

[viters]

@karlhorky Thank you for your quick response.

Further context: copying the Content-Security-Policy to the request is required for React to read the header there:

Given that, I think its just a matter of handling request and response headers separately:

const requestHeaders = new Headers(request.headers)
requestHeaders.set('x-nonce', nonce)
requestHeaders.set(
    'Content-Security-Policy',
    // Replace newline characters and spaces
    cspHeader.replace(/\s{2,}/g, ' ').trim()
)

const response = NextResponse.next({
    request: {
        headers: requestHeaders,
    },
})

response.headers.set('x-nonce', nonce)
response.headers.set(
    'Content-Security-Policy',
    // Replace newline characters and spaces
    cspHeader.replace(/\s{2,}/g, ' ').trim()
)

return response

I am not sure if thats proper way to propagate response headers, I am not experienced in middlewares.

[leerob]

https://github.com/vercel/next.js/pull/58300

[karlhorky]

@leerob in #58300 I think the part of the copying the request headers to the response (allowing reflection attacks) was not removed:

I think the problem here is line 31, no? I'm guessing that this copies the request headers to the newly-created response.

cc @viters

[karlhorky]

@leerob opened a new PR with this minimal change:

• https://github.com/vercel/next.js/pull/58304

[viters]

@karlhorky The problem is still present in documentation: https://nextjs.org/docs/pages/building-your-application/configuring/content-security-policy#adding-a-nonce-with-middleware

[github-actions[bot]]

This closed issue has been automatically locked because it had no new activity for 2 weeks. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.