From e6d79bb88ee47b9f56a84371f65a9eaac75c6fa4 Mon Sep 17 00:00:00 2001 From: Tim Neutkens Date: Thu, 11 Oct 2018 16:56:12 +0200 Subject: [PATCH] Remove pathname (#5428) Same as #5424 --- client/index.js | 7 +- server/document.js | 20 +++--- server/render.js | 7 +- test/integration/production/test/security.js | 68 +++++++++++++++++--- 4 files changed, 75 insertions(+), 27 deletions(-) diff --git a/client/index.js b/client/index.js index 4f7b83b9..8ca23c0d 100644 --- a/client/index.js +++ b/client/index.js @@ -24,7 +24,6 @@ const { props, err, page, - pathname, query, buildId, assetPrefix, @@ -83,7 +82,7 @@ export default async ({ Component = await pageLoader.loadPage(page) if (typeof Component !== 'function') { - throw new Error(`The default export is not a React Component in page: "${pathname}"`) + throw new Error(`The default export is not a React Component in page: "${page}"`) } } catch (error) { // This catches errors like throwing in the top level of a module @@ -92,7 +91,7 @@ export default async ({ await Loadable.preloadReady(dynamicIds || []) - router = createRouter(pathname, query, asPath, { + router = createRouter(page, query, asPath, { initialProps: props, pageLoader, App, @@ -141,7 +140,7 @@ export async function renderError (props) { // Otherwise, we need to call `getInitialProps` on `App` before mounting. const initProps = props.props ? props.props - : await loadGetInitialProps(App, {Component: ErrorComponent, router, ctx: {err, pathname, query, asPath}}) + : await loadGetInitialProps(App, {Component: ErrorComponent, router, ctx: {err, pathname: page, query, asPath}}) await doRender({...props, err, Component: ErrorComponent, props: initProps}) } diff --git a/server/document.js b/server/document.js index cfb38f36..52ab4dc0 100644 --- a/server/document.js +++ b/server/document.js @@ -101,8 +101,8 @@ export class Head extends Component { render () { const { head, styles, assetPrefix, __NEXT_DATA__ } = this.context._documentProps - const { page, pathname, buildId } = __NEXT_DATA__ - const pagePathname = getPagePathname(pathname) + const { page, buildId } = __NEXT_DATA__ + const pagePathname = getPagePathname(page) let children = this.props.children // show a warning if Head contains (only in development) @@ -186,21 +186,21 @@ export class NextScript extends Component { static getInlineScriptSource (documentProps) { const { __NEXT_DATA__ } = documentProps - const { page, pathname } = __NEXT_DATA__ - return `__NEXT_DATA__ = ${htmlescape(__NEXT_DATA__)};__NEXT_LOADED_PAGES__=[];__NEXT_REGISTER_PAGE=function(r,f){__NEXT_LOADED_PAGES__.push([r, f])}${page === '/_error' ? `;__NEXT_REGISTER_PAGE(${htmlescape(pathname)},function(){var e = new Error('Page does not exist: ${htmlescape(pathname)}');e.statusCode=404;return {error:e}})`:''}` + const { page } = __NEXT_DATA__ + return `__NEXT_DATA__ = ${htmlescape(__NEXT_DATA__)};__NEXT_LOADED_PAGES__=[];__NEXT_REGISTER_PAGE=function(r,f){__NEXT_LOADED_PAGES__.push([r, f])}` } render () { const { staticMarkup, assetPrefix, devFiles, __NEXT_DATA__ } = this.context._documentProps - const { page, pathname, buildId } = __NEXT_DATA__ - const pagePathname = getPagePathname(pathname) + const { page, buildId } = __NEXT_DATA__ + const pagePathname = getPagePathname(page) return <Fragment> {devFiles ? devFiles.map((file) => <script key={file} src={`${assetPrefix}/_next/${file}`} nonce={this.props.nonce} />) : null} {staticMarkup ? null : <script nonce={this.props.nonce} dangerouslySetInnerHTML={{ __html: NextScript.getInlineScriptSource(this.context._documentProps) }} />} - {page !== '/_error' && <script async id={`__NEXT_PAGE__${pathname}`} src={`${assetPrefix}/_next/static/${buildId}/pages${pagePathname}`} nonce={this.props.nonce} />} + {page !== '/_error' && <script async id={`__NEXT_PAGE__${page}`} src={`${assetPrefix}/_next/static/${buildId}/pages${pagePathname}`} nonce={this.props.nonce} />} <script async id={`__NEXT_PAGE__/_app`} src={`${assetPrefix}/_next/static/${buildId}/pages/_app.js`} nonce={this.props.nonce} /> <script async id={`__NEXT_PAGE__/_error`} src={`${assetPrefix}/_next/static/${buildId}/pages/_error.js`} nonce={this.props.nonce} /> {staticMarkup ? null : this.getDynamicChunks()} @@ -209,10 +209,10 @@ export class NextScript extends Component { } } -function getPagePathname (pathname) { - if (pathname === '/') { +function getPagePathname (page) { + if (page === '/') { return '/index.js' } - return `${pathname}.js` + return `${page}.js` } diff --git a/server/render.js b/server/render.js index 238d75eb..964017b0 100644 --- a/server/render.js +++ b/server/render.js @@ -89,14 +89,14 @@ async function doRender (req, res, pathname, query, { Component = Component.default || Component if (typeof Component !== 'function') { - throw new Error(`The default export is not a React Component in page: "${pathname}"`) + throw new Error(`The default export is not a React Component in page: "${page}"`) } App = App.default || App Document = Document.default || Document const asPath = req.url - const ctx = { err, req, res, pathname, query, asPath } - const router = new Router(pathname, query, asPath) + const ctx = { err, req, res, pathname: page, query, asPath } + const router = new Router(page, query, asPath) const props = await loadGetInitialProps(App, {Component, router, ctx}) const devFiles = buildManifest.devFiles const files = [ @@ -168,7 +168,6 @@ async function doRender (req, res, pathname, query, { __NEXT_DATA__: { props, // The result of getInitialProps page, // The rendered page - pathname, // The requested path query, // querystring parsed / passed by the user buildId, // buildId is used to facilitate caching of page bundles, we send it to the client so that pageloader knows where to load bundles assetPrefix: assetPrefix === '' ? undefined : assetPrefix, // send assetPrefix to the client side when configured, otherwise don't sent in the resulting HTML diff --git a/test/integration/production/test/security.js b/test/integration/production/test/security.js index 1f83c145..4c5fa07c 100644 --- a/test/integration/production/test/security.js +++ b/test/integration/production/test/security.js @@ -3,9 +3,21 @@ import { readFileSync } from 'fs' import { join } from 'path' -import { renderViaHTTP, waitFor } from 'next-test-utils' +import { renderViaHTTP, getBrowserBodyText, waitFor } from 'next-test-utils' import webdriver from 'next-webdriver' +// Does the same evaluation checking for INJECTED for 15 seconds, triggering every 500ms +async function checkInjected (browser) { + const start = Date.now() + while (Date.now() - start < 15000) { + const bodyText = await getBrowserBodyText(browser) + if (/INJECTED/.test(bodyText)) { + throw new Error('Vulnerable to XSS attacks') + } + await waitFor(500) + } +} + module.exports = (context) => { describe('With Security Related Issues', () => { it('should only access files inside .next directory', async () => { @@ -28,18 +40,56 @@ module.exports = (context) => { }) it('should prevent URI based XSS attacks', async () => { - const browser = await webdriver(context.appPort, '/\',document.body.innerHTML="HACKED",\'') - // Wait 5 secs to make sure we load all the client side JS code - await waitFor(5000) + const browser = await webdriver(context.appPort, '/\',document.body.innerHTML="INJECTED",\'') + await checkInjected(browser) + browser.quit() + }) - const bodyText = await browser - .elementByCss('body').text() + it('should prevent URI based XSS attacks using single quotes', async () => { + const browser = await webdriver(context.appPort, `/'-(document.body.innerHTML='INJECTED')-'`) + await checkInjected(browser) + browser.close() + }) - if (/HACKED/.test(bodyText)) { - throw new Error('Vulnerable to XSS attacks') - } + it('should prevent URI based XSS attacks using double quotes', async () => { + const browser = await webdriver(context.appPort, `/"-(document.body.innerHTML='INJECTED')-"`) + await checkInjected(browser) browser.close() }) + + it('should prevent URI based XSS attacks using semicolons and double quotes', async () => { + const browser = await webdriver(context.appPort, `/;"-(document.body.innerHTML='INJECTED')-"`) + await checkInjected(browser) + + browser.close() + }) + + it('should prevent URI based XSS attacks using semicolons and single quotes', async () => { + const browser = await webdriver(context.appPort, `/;'-(document.body.innerHTML='INJECTED')-'`) + await checkInjected(browser) + + browser.close() + }) + + it('should prevent URI based XSS attacks using src', async () => { + const browser = await webdriver(context.appPort, `/javascript:(document.body.innerHTML='INJECTED')`) + await checkInjected(browser) + + browser.close() + }) + + it('should prevent URI based XSS attacks using querystring', async () => { + const browser = await webdriver(context.appPort, `/?javascript=(document.body.innerHTML='INJECTED')`) + await checkInjected(browser) + + browser.close() + }) + + it('should prevent URI based XSS attacks using querystring and quotes', async () => { + const browser = await webdriver(context.appPort, `/?javascript="(document.body.innerHTML='INJECTED')"`) + await checkInjected(browser) + browser.close() + }) }) }