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

Content Security Policy (CSP) with next.js #256

Closed
dbo opened this issue Nov 14, 2016 · 59 comments
Closed

Content Security Policy (CSP) with next.js #256

dbo opened this issue Nov 14, 2016 · 59 comments

Comments

@dbo
Copy link
Contributor

dbo commented Nov 14, 2016

Edit: This issue is very old and has now been replaced with #23993.


I don't see a way using next.js with a sensible CSP employed, foremost because it's eval'ing the component code (unsafe-eval is highly discouraged w.r.t. XSS). In addition, the injected scripts page component scripts should get a computed hash, and an SRI hash should be added for the CDN/next.js script.

Excuse my ignorance about, but wouldn't it be better to use plain old script tag loading for the component module, props etc.?

@nkzawa
Copy link
Contributor

nkzawa commented Nov 15, 2016

I'm not familiar with CSP, but it seems next.js doesn't support CSP at all for now.
eval is used just because it's a more flexible way to execute codes compared to others.

@arunoda
Copy link
Contributor

arunoda commented Nov 15, 2016

CSP is kind a standard for web security and usually we disable eval with CSP. (and similar functionalities)
As a fix for this, we might need to load the page bundle as a JavaScript. Usually, loading scripts from the same domain is allowed with CSP. So, that won't be an issue.


That's being said, stored XSS is not possible with React unless you use dangerouslySetInnerHTML.

So, we are not in a huge security risk.

@arunoda
Copy link
Contributor

arunoda commented Nov 15, 2016

@dbo about this:

In addition, the injected scripts page component scripts should get a computed hash, and an SRI hash should be added for the CDN/next.js script.

I hope this is about another issue.
Shall we open a new issue for that?

@dbo
Copy link
Contributor Author

dbo commented Nov 15, 2016

@arunoda IMHO this is a blocker to seriously deploy next.js apps; CSP is a standard, eval discouraged. Avoiding dangerouslySetInnerHTML in your code is common practice, but what about 3rd party NPMs, e.g. older template engines. You'd have to audit all of your dependencies.

Regarding your latter question: I think this task should be about getting next.js running with strict CSP, i.e. preferably the hashes for the inline script as well as a SRI hash for the CDN-script could be generated (at build time?), so no need to allow any inline-script, CDN script-src.

@rauchg
Copy link
Member

rauchg commented Nov 16, 2016

@dbo just so I understand the attack surface better: why would you use a template engine in this context?

@dbo
Copy link
Contributor Author

dbo commented Nov 16, 2016

As I've written, my primary code can (and should) avoid dangerouslySetInnerHTML and also doesn't need a templating engine, since it's react. But any 3rd party NPM (and transitive dependencies) that I use might do so, especially any 3rd party react component might use dangerouslySetInnerHTML.

Also look at this from a different angle: Any serious customer would require you to do a security audit/proof of the site you've built. Given a decent browser baseline, it's much easier and less effort to do that in context of a CSP.

@rauchg
Copy link
Member

rauchg commented Nov 17, 2016

Definitely. I'll keep this open. Consider this approach:
#253 (comment)

The problem is that it introduces a new roundtrip, however.

@dbo
Copy link
Contributor Author

dbo commented Nov 22, 2016

@rauchg reads sensible
@nkzawa To be honest, I consider this a major blocker for any production site. So this doesn't sound like an enhancement to me.

@rauchg
Copy link
Member

rauchg commented Nov 22, 2016

What you're saying is that you want a protection from yourself (unaudited code that you might include in your own codebase). That by definition is a security enhancement, not a hole.

I think this is important to address, and we're thinking about how to best do it without a tradeoff in performance.

@dbo
Copy link
Contributor Author

dbo commented Nov 22, 2016

@rauchg Of course this is not about a security hole, but a very important security enhancement, it's highly recommended and standardised practice for good reason.
Regarding your point of "protection from yourself", please keep in mind that a lot of developers simply don't have

  • the time
  • the experience

to thoroughly audit all of their transitive dependencies. I simply don't expect it to happen.

@rauchg
Copy link
Member

rauchg commented Nov 22, 2016

Absolutely. Was just pointing out that @nkzawa picked the right label, and
that we consider it very important still.

On Tue, Nov 22, 2016 at 9:53 AM ǝlzlǝoq lǝᴉuɐp ツ [email protected]
wrote:

@rauchg https://github.com/rauchg Of course this is not about a
security hole, but a very important security enhancement, it's highly
recommended and standardised practice for good reason.
Regarding your point of "protection from yourself", please keep in mind
that a lot of developers simply don't have

  • the time
  • the experience
    to thoroughly audit all of their transitive dependencies. I simply
    don't expect it to happen.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#256 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAAy8Q8YhTOBrwAaIk-XR9x9iSbGkZHQks5rAx4hgaJpZM4Kx9aI
.

@aesopwolf
Copy link

CSP headers are definitely a high priority in my opinion. However, it's possible to use a meta tag as an alternative for the time being.

https://developers.google.com/web/fundamentals/security/csp/#the_meta_tag

@dbo
Copy link
Contributor Author

dbo commented Dec 13, 2016

@aesopwolf Using a meta tag as an alternative to HTTP headers is not the issue here, it will prevent the page from working, too, unless you allow unsafe-eval.

@aesopwolf
Copy link

@dbo my apologies, I skimmed over the thread too fast.

@rauchg Is there anything we can do to help fast-lane the SRI hash for the CDN script? My company is in the final phases of a new registration app built on next.js and we expect to see 150k signups in January so having the CDN would definitely be nice to have.

@rauchg
Copy link
Member

rauchg commented Dec 24, 2016

@aesopwolf as of 2.0+ there won't be a "global build" anymore, so it's not possible for us to provide a unique CDN. Each site generates its own minimal build of the framework

@rauchg rauchg closed this as completed Dec 24, 2016
@rauchg rauchg reopened this Dec 24, 2016
@aesopwolf
Copy link

@rauchg thanks for the update, that sounds like the best thing to do anyways. It moves work off zeit's end and gives developers full control of their own caching strategy 👍

@rauchg
Copy link
Member

rauchg commented Dec 24, 2016

Yeah. The goal was to provide an optimized baseline so that Next could be cached across several internet properties. But in the end, there are many slices and versions of Next across those properties, so it's a difficult endeavor.

@dav-is
Copy link
Contributor

dav-is commented Sep 24, 2017

Update on this?

@pozylon
Copy link

pozylon commented Oct 10, 2017

Workaround 😂:

<Head>
      <meta httpEquiv="Content-Security-Policy" content="default-src * 'self' data: 'unsafe-inline' 'unsafe-eval' *" />    
</Head>

@dav-is
Copy link
Contributor

dav-is commented Oct 10, 2017

That completely defeats the purpose of CSP.

@jeffhandley
Copy link

jeffhandley commented Oct 10, 2017

Edit: I dug up our internal documentation on this topic and I have added some clarification, references, and corrections to my original comment

The approach I've taken on projects where we want server-side rendering of React components and then client-side re-rendering (progressive rendering), where data needs to be serialized and then deserialized as part of the payload is to:

Emit the data payload into a <noscript> tag as a JSON-stringified payload. Then, the library code that relies on the data finds the element by id and parses its content into the necessary variables. In the case of next.js, it would likely be something like this:

<noscript id="__NEXT_DATA__">{props:{...}, ...}</noscript>

We don't just blindly use document.getElementById though. Instead we use querySelectorAll and ensure that there's only a single element on the page with that id. That way, even if there is a dangerouslySetInnerHtml vulnerability elsewhere on the page, this specific feature of loading data from the server would not be vulnerable. We use the textContent property of the element once we've found it.

Using this approach, we serialize our store and other data from the server into these <noscript> tags, load them from our library code, and we get to where we have zero inline <script> tags. Every <script> has a source coming from the same domain and we can meet the CSP requirements.

You can read the OWASP coverage of this topic here:
https://www.owasp.org/index.php/XSS_(Cross_Site_Scripting)_Prevention_Cheat_Sheet#HTML_entity_encoding

Our approach is aligned with what they document there, but we use that extra check that there's a single element matching the ID we're looking for, and we put the data into a <noscript> element instead of a <div>.

@jeffhandley
Copy link

Here's what I spiked out to see if this approach would work. A modified NextScript component's render function:

  render () {
    const { staticMarkup, __NEXT_DATA__, chunks } = this.context._documentProps
    const { pathname, nextExport, buildId, assetPrefix } = __NEXT_DATA__
    const pagePathname = getPagePathname(pathname, nextExport)

    __NEXT_DATA__.chunks = chunks

    return <div>
      {staticMarkup ? null : (
        <div>
          <noscript id='__NEXT_DATA__' dangerouslySetInnerHTML={{ __html: htmlescape(JSON.stringify(__NEXT_DATA__)) }} />
          <script type='text/javascript' src={`${assetPrefix}/static/next-script.js?${buildId}`} />
        </div>
      )}
      <script async id={`__NEXT_PAGE__${pathname}`} type='text/javascript' src={`${assetPrefix}/_next/${buildId}/page${pagePathname}`} />
      <script async id={`__NEXT_PAGE__/_error`} type='text/javascript' src={`${assetPrefix}/_next/${buildId}/page/_error/index.js`} />
      {staticMarkup ? null : this.getDynamicChunks()}
      {staticMarkup ? null : this.getScripts()}
    </div>
  }

And then I had a next-script.js file with the following (that would ideally move into the next main module before __NEXT_DATA is expected to be there):

var nextData = document.querySelectorAll('noscript#__NEXT_DATA__');

if (nextData.length === 1) {
    window.__NEXT_DATA__ = JSON.parse(nextData[0].textContent);
    window.module = {};
    window.__NEXT_LOADED_PAGES__ = [];
    window.__NEXT_LOADED_CHUNKS__ = [];
}

function __NEXT_REGISTER_PAGE(route, fn) {
    __NEXT_LOADED_PAGES__.push({ route: route, fn: fn })
}

function __NEXT_REGISTER_CHUNK(chunkName, fn) {
    __NEXT_LOADED_CHUNKS__.push({ chunkName: chunkName, fn: fn })
}

With that in place, I added the following express middleware to my custom server:

function contentSecurityPolicy(req, res, next) {
    res.setHeader('Content-Security-Policy', 'script-src \'self\'');
    next();
}

Running the app, I get an error:

ndefined is not iterable!
TypeError: undefined is not iterable!
    at e.exports.n.getIterator (http://localhost:3000/_next/6c43080790430acdd9608370496ef26e/app.js:7:30409)
    at http://localhost:3000/_next/6c43080790430acdd9608370496ef26e/app.js:8:3491
    at r (http://localhost:3000/_next/6c43080790430acdd9608370496ef26e/app.js:7:22793)
    at Generator._invoke (http://localhost:3000/_next/6c43080790430acdd9608370496ef26e/app.js:7:23838)
    at Generator.e.(anonymous function) [as next] (http://localhost:3000/_next/6c43080790430acdd9608370496ef26e/app.js:7:22972)
    at r (http://localhost:3000/_next/6c43080790430acdd9608370496ef26e/app.js:2:3765)
    at http://localhost:3000/_next/6c43080790430acdd9608370496ef26e/app.js:2:3913
    at new Promise (<anonymous>)
    at new t (http://localhost:3000/_next/6c43080790430acdd9608370496ef26e/app.js:2:1955)
    at http://localhost:3000/_next/6c43080790430acdd9608370496ef26e/app.js:2:3707
(anonymous) @ app.js:8
Promise rejected (async)
132 @ app.js:8
n @ app.js:1
131 @ app.js:8
n @ app.js:1
window.webpackJsonp @ app.js:1
(anonymous) @ app.js:8

This is preventing modules from getting loaded, but I've not been able to track down what is causing this.

@Potherca
Copy link

Potherca commented Oct 26, 2017

Just to be clear... Is this the code that causes all the trouble?

https://github.com/zeit/next.js/blob/9320d9f006164f2e997982ce76e80122049ccb3c/server/build/babel/plugins/handle-import.js#L14

'Cause it does not look like that needs to be evaled at all.[1]


1 I get the distinct impression that I might be missing something here... 😕

@jeffhandley
Copy link

It does look like the eval is an issue for CSP too, but the initial reports for this issue relate to using inline script code.

https://www.owasp.org/index.php/Content_Security_Policy_Cheat_Sheet#Refactoring_inline_code

Instead of having <script> tags with executable code inside, the executable code needs to be moved into <script src> tags--no dynamic executable JavaScript can be used on the page. For universal rendering, that means that initial props/state data needs to be sent to the browser differently--like I illustrated with the <noscript> approach.

After fixing that, the eval issue might also need to be fixed though.

You can use Report-Only headers or CSP tester tools to test CSP compatibility.

@jwb
Copy link

jwb commented Jan 17, 2018

I heard from @rauchg that this issue should be resolved in the current release. Is that accurate?

@dav-is
Copy link
Contributor

dav-is commented Aug 28, 2018

If my PR gets merged soon, CSP will be as simple as an extra line in the config file. :)

@davidcunha
Copy link

davidcunha commented Sep 24, 2018

I tested with next.js 7.0 and without adding 'unsafe-eval' to my CSP config I get:

Uncaught EvalError: Refused to evaluate a string as JavaScript because 'unsafe-eval' is not an allowed source of script in the following Content Security Policy directive:

What's the plan to not require any workaround or adding 'unsafe-eval' to the config which is definitely against CSP?

@dav-is
Copy link
Contributor

dav-is commented Sep 24, 2018

@davidcunha Hot Module Replacement (HMR) requires eval. This is only run in dev mode and unsafe-eval will be automatically added in dev mode.

In production, this isn’t an issue. Also, my PR didn’t make it into 7.0, if you thought it did.

@davidcunha
Copy link

davidcunha commented Sep 24, 2018

Yes indeed, it's not an issue in production. I'm getting only this error while running in dev mode.

@Jero786
Copy link
Contributor

Jero786 commented Jan 11, 2019

There is some update from this PR? there would be awesome to be in the next release version.

ijjk pushed a commit to ijjk/next.js that referenced this issue Apr 13, 2019
@matoous
Copy link

matoous commented Aug 22, 2019

Hello! Any news?

@timneutkens
Copy link
Member

@matoous any news on what?

https://nextjs.org/blog/next-8#removed-inline-javascript

@javidjamae
Copy link

Looks like you can include Content Security Policy headers in NextJS 9.5

https://nextjs.org/blog/next-9-5#headers
https://nextjs.org/docs/api-reference/next.config.js/headers

@OZZlE
Copy link

OZZlE commented Oct 12, 2020

This worked for me in production mode nextjs, next.config.js:

const withImages = require('next-images')
module.exports = withImages({
  async headers() {
    return [
      {
        source: '/:path*',
        headers: [
          {
            // // https://developer.mozilla.org/en-US/docs/Web/HTTP/CSP
            key: 'Content-Security-Policy',
            value:
              "default-src [your-domains...] 'self' data: 'unsafe-inline'"
          }
        ]
      }
    ]
  }
})

If you don't use next-images then you can just remove that, export the object only.

We are still on [email protected]

@ivan-kleshnin
Copy link
Contributor

ivan-kleshnin commented Nov 4, 2020

@OZZlE this adds Content-Security-Policy to responses of static assets, non only HTML.
Which hurts performance (slightly).

I believe that:

// pages/_document.js
...
<Head>
  <meta httpEquiv="Content-Security-Policy" content={csp} />
</Head>

is a better approach. You don't have to mess with regexp trying to detect statics. Also it's more obvious in _document than in next.config.js (which is often long and overcomplicated enough) IMO.

@OZZlE
Copy link

OZZlE commented Nov 4, 2020

@ivan-kleshnin probably you are correct but I was just handed a pre-existing project and they have multiple <Head> components for some reason.. I guess the CSP could be fetched from the same place still though :) Their next.config.js was almost empty though in this case :)

@bozdoganCihangir
Copy link

bozdoganCihangir commented Nov 30, 2020

All works fine after removing the "helmet" npm package.

@trezy
Copy link

trezy commented Dec 17, 2020

For anybody else finding this issue while trying to figure out CSP stuff, I ended up building my own library to handle it. I'd encourage you all to check it out, too.

https://npmjs.com/package/next-safe

@JustFly1984
Copy link

@trezy is it possible to use your package with CSP nonce?

@trezy
Copy link

trezy commented Dec 19, 2020

@JustFly1984 not yet, no. The nonce from Next is only exposed inside the app, but next-safe runs at config time. It's something I've been looking into, but there's not a good solution for using CSP nonces with Next unless you set your CSP via the meta tags, which is substantially less secure than using headers.

@JustFly1984
Copy link

JustFly1984 commented Dec 20, 2020

I need a solution for CSP with nonce. Currently we found one architecture with CSP nonce , Gatsby.js and AWS API gateway, but that architecture means that we lost CDN capabilities of AWS Cloudfront.

@etienne-martin
Copy link

@OZZlE this adds Content-Security-Policy to responses of static assets, non only HTML.
Which hurts performance (slightly).

I believe that:

// pages/_document.js
...
<Head>
  <meta httpEquiv="Content-Security-Policy" content={csp} />
</Head>

is a better approach. You don't have to mess with regexp trying to detect statics. Also it's more obvious in _document than in next.config.js (which is often long and overcomplicated enough) IMO.

@ivan-kleshnin Keep in mind that the CSP policy only applies to content found after the meta tag is processed, so you should keep it towards the top of your document. I learned that the hard way.

@alex-r89
Copy link

Sorry for commenting on a closed issue, but I (and a number of others) seem to be running into this issue with CSP errors using a nonce - it doesnt seem to work with inline styles among other things.

@dbo is there a guide or example of implementing a SHA hash at all? I had a look at your PR but it didn't make much sense to me 🙁

@YoannBuzenet
Copy link

Hello, I have the same problem - is there some docs around this topic ?

@dbo
Copy link
Contributor Author

dbo commented May 4, 2021

I never got inline styles to work with my CSP, the case back then was emitting a hash for inline scripts (NextScript.getInlineScriptSource). This is not needed any longer, since the bootstrap part is no longer inlined (just picks up the __NEXT_DATA__ which is application/json). Aside of that I am not using nonces, since I am a big fan of CDNs and static export, which also eliminates another class of security problem: running a server ;-)

@leerob
Copy link
Member

leerob commented Jun 23, 2021

Related #23993

@balazsorban44
Copy link
Member

This issue has been automatically locked due to no recent activity. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@vercel vercel locked as resolved and limited conversation to collaborators Jan 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests