Add Trusted Types policy

[TyMick]

Resolves #13228, at least for production builds (doesn't fix the bug in the dev server, seemingly due to webpack), though a security review is needed.

Trusted Types is a new experimental DOM API created to combat DOM XSS, supported now in Chrome 83, with a polyfill available for other browsers. @tipra34 noticed that when requiring Trusted Types for scripts via <meta httpEquiv="Content-Security-Policy" content="require-trusted-types-for 'script';" />, it broke links (using next/link) in Chrome 83: hovering over links would show the correct target URL, but clicking the link would do nothing. They ended up determining that the offending script src assignment took place in PageLoader.loadScript().

This update adds a new custom Trusted Types policy by placing the following at the top of page-loader.js:

let trustedTypesPolicy = undefined
if (window?.trustedTypes?.createPolicy) {
  trustedTypesPolicy = window.trustedTypes.createPolicy('next-trusted-types', {
    // Needs security review regarding DOM XSS
    createHTML(dirty) {
      return dirty.replace(/</g, '&lt;') // Escapes `<` characters to prevent the creation of new HTML elements
    },
    createScriptURL(dirty) {
      return new URL(dirty.replace(/</g, '&lt;'), document.baseURI)
    },
  })
}

...and changing the script.src assignment in loadScript() to

script.src = trustedTypesPolicy
  ? trustedTypesPolicy.createScriptURL(url)
  : url

I'm not certain how secure .replace(/</g, '&lt;') is as a security policy, so more experienced eyes there would be appreciated. Alternatively, we could use DOMPurify instead of creating a custom policy, and just

import DOMPurify from 'dompurify'
script.src = DOMPurify.sanitize(url, { RETURN_TRUSTED_TYPE: true })

...but that adds an extra dependency we aren't already using.

Also, this only fixes the bug for production builds (including when server-rendering). Using the dev server, the entire page breaks and the console points to a couple of different places where webpack is using eval(). Haven't done enough investigation yet to determine if that's something that we can fix on our end or that webpack needs to fix.

[timneutkens]

This needs a review from @spanicker's team 👍

[prateekbh]

is this even needed for this particular case

[rictic]

Looks like it isn't used, so the createHTML method here can be dropped.

[prateekbh]

I wonder if encodeURI is a better tool for createScript instead of the /</ replacement.

[rictic]

Please forgive the drive-by review by a rando. For context, I've worked on integrating Trusted Types into the lit-html project. Hopefully this advice is helpful :)

[rictic]

Looks like it isn't used, so the createHTML method here can be dropped.

[rictic]

So, the attack you're guarding against with createScriptUrl is an untrusted/malicious URL, which can execute arbitrary code (e.g. by loading a script from a hostile origin that exfiltrates user data, mines bitcoins, etc)

It looks like you construct the URLs in two places, at line 263 and line 103. If you are confident that the data used to construct those URLs can't come from untrusted sources, then you could have a createScriptURL function that just returns its argument, and call policy.createScriptUrl there (with comments about your assumptions).

Then, because policy isn't exported, you and your users can reduce the amount of code that needs to be subject to careful security review to just those where the policy is used in this file.

[Siegrift]

Hi, I am an intern at Google and last year I focused on trusted types (TT) integration to client side frameworks and I also created an integration for nextjs but I didn't create a PR as the code changes were needed in React and Webpack.

In your PR you missed one execution in head-manager. The code will be executed if you include script tag in the nextjs/head or an iframe with srcdoc attribute. For nextjs to be TT compliant in development mode you need to patch prerender-indicator.js and dev-build-watcher.js as well.

(Note: The PR's and examples and PR's I provide in next sections might be a bit outdated).

Unfortunately (as you already know) for pleasant development with trusted types enabled you need to integrate with Webpack and React as well. There are open PRs for Webpack (webpack/webpack#9856 and webpack/webpack#9861) and React accepted TT behind feature flag (facebook/react#16157). Adding proper support for trusted types would require finishing the integration in these dependencies (merging webpack, removing the feature flag in React or using the build with TT). Using latest version of React (v16.13.1) without TT enabled, you are not able to use dangerouslySetInnerHTML nor any potentially unsafe attributes/properties (e.g iframe srcdoc) because the values will be stringified along the way to the sink reaching it as a plain string. Soon it might will be possible to use at least dangerouslySetInnerHTML (facebook/react#17773).

As nextjs can render the code on server side, it is possible to introduce reflected XSS (example). We also experimented with adding TT for React on server using the polyfill, however this is not in the threat model of TT.

Last thing, if you want to use TT in dev mode as well, you can use CSP in report only mode. Unfortunately, that is the only way as you can't use TT default policy, because Webpack code is executed before user code.

If you are interested in adding full trusted types support, we are glad to help you with integration or reviews.

[koto]

FWIW since next.js is now in Typescript, we can leverage tsec to statically discover Trusted Types violations.

Where's what it shows now (there's at least one false positive violation with setTimeout):

$ cd pacakges/next
$ yarn add tsec --dev
$ yarn run tsec -p tsconfig.json
yarn run v1.22.10
$ /Users/koto/dev/next.js/node_modules/.bin/tsec -p tsconfig.json
client/route-loader.ts:147:5 - error TS21228: [ban-script-src-assignments] Do not assign variables to HTMLScriptElement#src, as this can lead to XSS.

147     script.src = src
        ~~~~~~~~~~~~~~~~
lib/recursive-delete.ts:5:25 - error TS21228: [ban-window-stringfunctiondef] Do not use setTimeout, as calling it with a string argument can cause code-injection security vulnerabilities.

5 const sleep = promisify(setTimeout)
                          ~~~~~~~~~~
client/head-manager.ts:25:7 - error TS21228: [ban-element-setattribute] Do not use Element#setAttribute or similar APIs, as this can lead to XSS.

25       el.setAttribute(attr, props[p])
         ~~~~~~~~~~~~~~~
client/head-manager.ts:31:5 - error TS21228: [ban-element-innerhtml-assignments] Assigning directly to Element#innerHTML can result in XSS vulnerabilities.

31     el.innerHTML = dangerouslySetInnerHTML.__html || ''
       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
client/script.tsx:79:5 - error TS21228: [ban-element-innerhtml-assignments] Assigning directly to Element#innerHTML can result in XSS vulnerabilities.

79     el.innerHTML = dangerouslySetInnerHTML.__html || ''
       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
client/script.tsx:81:5 - error TS21228: [ban-script-content-assignments] Do not assign values to HTMLScriptElement#text or HTMLScriptElement#textContent, as this can lead to XSS.

 81     el.textContent =
        ~~~~~~~~~~~~~~~~
 82       typeof children === 'string'
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
...
 85         ? children.join('')
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~
 86         : ''
    ~~~~~~~~~~~~
client/script.tsx:88:5 - error TS21228: [ban-script-src-assignments] Do not assign variables to HTMLScriptElement#src, as this can lead to XSS.

88     el.src = src
       ~~~~~~~~~~~~
client/script.tsx:97:5 - error TS21228: [ban-element-setattribute] Do not use Element#setAttribute or similar APIs, as this can lead to XSS.

97     el.setAttribute(attr, value)
       ~~~~~~~~~~~~~~~

React by now should support TT in dangerouslySetInnerHtml, Webpack 5 is also OK via output.trustedTypes, so the integration should be relatively simple.

[Siegrift]

Hi everyone :)

I'd like to revisit this issue and make it possible to use Trusted Types in nextJS project - for both production and environment. I am working on it in my free time with no fixed schedule so I can't promise anything.

So far I have a runnable nextJS project with Trusted Types enabled. So far, I've:

• introduced a policy which fixes the issues reported by tsec - see Add Trusted Types policy #13509 (comment)
• in the application I allowed eval sink - This is likely caused by webpack, or better said some webpack plugin. I am using webpack 5, which supports trusted types but it seems it's not enough (I'll need to investigate more)

You can see the code in https://github.com/Siegrift/next.js/tree/tt-fixes and https://github.com/Siegrift/master-thesis/tree/master/code/tt-example-app

[timneutkens]

👋 Someone from the Chrome team is going to start working on adding Trusted Types to Next.js early January. I'm going to close this PR as we can't land it as-is. I've opened a new issue for implementing trusted types here to track it: #32209