From 0dd62111f63770b213f45e2d199a0494f20415f3 Mon Sep 17 00:00:00 2001 From: Justin Goping <32006038+jgoping@users.noreply.github.com> Date: Wed, 4 May 2022 20:11:36 -0400 Subject: [PATCH] Fix various Trusted Types violations without use of policy (#34726) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Linked to issue #32209. ## Feature - [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. - [x] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Documentation added - [ ] Telemetry added. In case of a feature if it's used or not. - [ ] Errors have helpful link attached, see `contributing.md` ## Documentation There are three Trusted Types violations that are fixed in this PR: ### 1. ban-element-innerhtml-assignments: maintain--tab-focus.ts The innerHTML assignment here is unsafe as a string is being used that could contain an XSS attack. The solution chosen was to replace the string containing HTML with programmatically-created DOM elements. This removes the Trusted Types violation as there is no longer a string passed in that can contain an XSS attack. Notes on solution: - The `` tag is omitted completely since the original snippet returns fragment.firstChild.firstChild. The first firstChild omits the `
`, and the second firstChild omits the ``, so to remove unnecessary code the created elements start at the foreignObject level. - The reason createElementNS is used instead of createElement is because the ‘foreignObject’ element is a separate namespace from the default HTML elements. The documentation for this command is found [here](https://developer.mozilla.org/en-US/docs/Web/API/Document/createElementNS). The code was tested to be equivalent by rendering both the original code and the re-written code in a browser to see if they evaluate to the same thing in the DOM. The DOM elements styles were then compared to ensure that they were identical. ### 2. ban-window-stringfunctiondef: packages/next/lib/recursive-delete.ts The setTimeout function caused a Trusted Types violation because if a string is passed in as the callback, XSS can occur. The solution to this problem is to ensure that only function callbacks can be passed to setTimeout. There is only one call to the sleep function and it does not involve a string callback, so this can be enforced without breaking the application logic. In the process of doing this, promisify has been removed and the promise has been created explicitly. The code was tested in a sample application to ensure it behaved as expected. ### 3. ban-window-stringfunctiondef: packages/next/client/dev/fouc.ts This file also uses setTimeout, so the call was wrapped in a `safeSetTimeout` call that specifies that the callback argument is not a string. --- packages/next/client/dev/fouc.ts | 5 ++++- packages/next/lib/recursive-delete.ts | 4 ++-- .../components/Overlay/maintain--tab-focus.ts | 15 +++++++++++---- tsec-exemptions.json | 9 ++------- 4 files changed, 19 insertions(+), 14 deletions(-) diff --git a/packages/next/client/dev/fouc.ts b/packages/next/client/dev/fouc.ts index 07e09f2c4bc1..bca480e22719 100644 --- a/packages/next/client/dev/fouc.ts +++ b/packages/next/client/dev/fouc.ts @@ -1,9 +1,12 @@ +// This wrapper function is used to avoid raising a Trusted Types violation. +const safeSetTimeout = (callback: () => void) => setTimeout(callback) + // This function is used to remove Next.js' no-FOUC styles workaround for using // `style-loader` in development. It must be called before hydration, or else // rendering won't have the correct computed values in effects. export function displayContent(): Promise { return new Promise((resolve) => { - ;(window.requestAnimationFrame || setTimeout)(function () { + ;(window.requestAnimationFrame || safeSetTimeout)(function () { for ( var x = document.querySelectorAll('[data-next-hide-fouc]'), i = x.length; diff --git a/packages/next/lib/recursive-delete.ts b/packages/next/lib/recursive-delete.ts index 76ed97712fb3..ccd443c4c2fa 100644 --- a/packages/next/lib/recursive-delete.ts +++ b/packages/next/lib/recursive-delete.ts @@ -1,9 +1,9 @@ import { Dirent, promises } from 'fs' import { join, isAbsolute, dirname } from 'path' -import { promisify } from 'util' import isError from './is-error' -const sleep = promisify(setTimeout) +const sleep = (timeout: number) => + new Promise((resolve) => setTimeout(resolve, timeout)) const unlinkPath = async (p: string, isDir = false, t = 1): Promise => { try { diff --git a/packages/react-dev-overlay/src/internal/components/Overlay/maintain--tab-focus.ts b/packages/react-dev-overlay/src/internal/components/Overlay/maintain--tab-focus.ts index 879db0409401..a9ef31097ce6 100644 --- a/packages/react-dev-overlay/src/internal/components/Overlay/maintain--tab-focus.ts +++ b/packages/react-dev-overlay/src/internal/components/Overlay/maintain--tab-focus.ts @@ -804,11 +804,18 @@ var focusSummary = { } function makeFocusableForeignObject() { - var fragment = document.createElement('div') - fragment.innerHTML = - '\n \n ' + // Constructs + // without raising a Trusted Types violation + var foreignObject = document.createElementNS( + 'http://www.w3.org/2000/svg', + 'foreignObject' + ) + foreignObject.width.baseVal.value = 30 + foreignObject.height.baseVal.value = 30 + foreignObject.appendChild(document.createElement('input')) + foreignObject.lastChild.type = 'text' - return fragment.firstChild.firstChild + return foreignObject } function focusSvgForeignObjectHack(element) { diff --git a/tsec-exemptions.json b/tsec-exemptions.json index acbcecdaa91c..ed01f28e893e 100644 --- a/tsec-exemptions.json +++ b/tsec-exemptions.json @@ -1,8 +1,7 @@ { "ban-element-innerhtml-assignments": [ "packages/next/client/head-manager.ts", - "packages/next/client/script.tsx", - "packages/react-dev-overlay/src/internal/components/Overlay/maintain--tab-focus.ts" + "packages/next/client/script.tsx" ], "ban-element-setattribute": [ "packages/next/client/head-manager.ts", @@ -10,9 +9,5 @@ ], "ban-script-content-assignments": ["packages/next/client/script.tsx"], "ban-script-src-assignments": ["packages/next/client/script.tsx"], - "ban-trustedtypes-createpolicy": ["packages/next/client/trusted-types.ts"], - "ban-window-stringfunctiondef": [ - "packages/next/lib/recursive-delete.ts", - "packages/next/client/dev/fouc.ts" - ] + "ban-trustedtypes-createpolicy": ["packages/next/client/trusted-types.ts"] }