From 095a8502f4e827351a983ff7c29980e2ebb34711 Mon Sep 17 00:00:00 2001 From: skooch Date: Thu, 16 Dec 2021 14:38:26 +1100 Subject: [PATCH] Backport security fix from https://github.com/vercel/next.js/pull/32092 --- packages/next/server/next-server.ts | 291 ++++++++++--------- test/integration/production/test/security.js | 26 ++ 2 files changed, 177 insertions(+), 140 deletions(-) diff --git a/packages/next/server/next-server.ts b/packages/next/server/next-server.ts index b4ce74c36472fcb..7780592bb07388e 100644 --- a/packages/next/server/next-server.ts +++ b/packages/next/server/next-server.ts @@ -333,184 +333,195 @@ export default class Server { res: ServerResponse, parsedUrl?: UrlWithParsedQuery ): Promise { - const urlParts = (req.url || '').split('?') - const urlNoQuery = urlParts[0] - - if (urlNoQuery?.match(/(\\|\/\/)/)) { - const cleanUrl = normalizeRepeatedSlashes(req.url!) - res.setHeader('Location', cleanUrl) - res.setHeader('Refresh', `0;url=${cleanUrl}`) - res.statusCode = 308 - res.end(cleanUrl) - return - } + try { + const urlParts = (req.url || '').split('?') + const urlNoQuery = urlParts[0] + + if (urlNoQuery?.match(/(\\|\/\/)/)) { + const cleanUrl = normalizeRepeatedSlashes(req.url!) + res.setHeader('Location', cleanUrl) + res.setHeader('Refresh', `0;url=${cleanUrl}`) + res.statusCode = 308 + res.end(cleanUrl) + return + } - setLazyProp({ req: req as any }, 'cookies', getCookieParser(req.headers)) + setLazyProp({ req: req as any }, 'cookies', getCookieParser(req.headers)) - // Parse url if parsedUrl not provided - if (!parsedUrl || typeof parsedUrl !== 'object') { - const url: any = req.url - parsedUrl = parseUrl(url, true) - } - const { basePath, i18n } = this.nextConfig + // Parse url if parsedUrl not provided + if (!parsedUrl || typeof parsedUrl !== 'object') { + const url: any = req.url + parsedUrl = parseUrl(url, true) + } + const { basePath, i18n } = this.nextConfig - // Parse the querystring ourselves if the user doesn't handle querystring parsing - if (typeof parsedUrl.query === 'string') { - parsedUrl.query = parseQs(parsedUrl.query) - } + // Parse the querystring ourselves if the user doesn't handle querystring parsing + if (typeof parsedUrl.query === 'string') { + parsedUrl.query = parseQs(parsedUrl.query) + } - ;(req as any).__NEXT_INIT_URL = req.url - ;(req as any).__NEXT_INIT_QUERY = Object.assign({}, parsedUrl.query) + ;(req as any).__NEXT_INIT_URL = req.url + ;(req as any).__NEXT_INIT_QUERY = Object.assign({}, parsedUrl.query) - const url = parseNextUrl({ - headers: req.headers, - nextConfig: this.nextConfig, - url: req.url?.replace(/^\/+/, '/'), - }) + const url = parseNextUrl({ + headers: req.headers, + nextConfig: this.nextConfig, + url: req.url?.replace(/^\/+/, '/'), + }) - if (url.basePath) { - ;(req as any)._nextHadBasePath = true - req.url = req.url!.replace(basePath, '') || '/' - } + if (url.basePath) { + ;(req as any)._nextHadBasePath = true + req.url = req.url!.replace(basePath, '') || '/' + } - if ( - this.minimalMode && - req.headers['x-matched-path'] && - typeof req.headers['x-matched-path'] === 'string' - ) { - const reqUrlIsDataUrl = req.url?.includes('/_next/data') - const matchedPathIsDataUrl = - req.headers['x-matched-path']?.includes('/_next/data') - const isDataUrl = reqUrlIsDataUrl || matchedPathIsDataUrl - - let parsedPath = parseUrl( - isDataUrl ? req.url! : (req.headers['x-matched-path'] as string), - true - ) - const { pathname, query } = parsedPath - let matchedPathname = pathname as string + if ( + this.minimalMode && + req.headers['x-matched-path'] && + typeof req.headers['x-matched-path'] === 'string' + ) { + const reqUrlIsDataUrl = req.url?.includes('/_next/data') + const matchedPathIsDataUrl = + req.headers['x-matched-path']?.includes('/_next/data') + const isDataUrl = reqUrlIsDataUrl || matchedPathIsDataUrl + + let parsedPath = parseUrl( + isDataUrl ? req.url! : (req.headers['x-matched-path'] as string), + true + ) + const { pathname, query } = parsedPath + let matchedPathname = pathname as string - let matchedPathnameNoExt = isDataUrl - ? matchedPathname.replace(/\.json$/, '') - : matchedPathname + let matchedPathnameNoExt = isDataUrl + ? matchedPathname.replace(/\.json$/, '') + : matchedPathname - if (i18n) { - const localePathResult = normalizeLocalePath( - matchedPathname || '/', - i18n.locales - ) + if (i18n) { + const localePathResult = normalizeLocalePath( + matchedPathname || '/', + i18n.locales + ) - if (localePathResult.detectedLocale) { - parsedUrl.query.__nextLocale = localePathResult.detectedLocale + if (localePathResult.detectedLocale) { + parsedUrl.query.__nextLocale = localePathResult.detectedLocale + } } - } - if (isDataUrl) { - matchedPathname = denormalizePagePath(matchedPathname) - matchedPathnameNoExt = denormalizePagePath(matchedPathnameNoExt) - } - - const pageIsDynamic = isDynamicRoute(matchedPathnameNoExt) - const combinedRewrites: Rewrite[] = [] + if (isDataUrl) { + matchedPathname = denormalizePagePath(matchedPathname) + matchedPathnameNoExt = denormalizePagePath(matchedPathnameNoExt) + } - combinedRewrites.push(...this.customRoutes.rewrites.beforeFiles) - combinedRewrites.push(...this.customRoutes.rewrites.afterFiles) - combinedRewrites.push(...this.customRoutes.rewrites.fallback) + const pageIsDynamic = isDynamicRoute(matchedPathnameNoExt) + const combinedRewrites: Rewrite[] = [] - const utils = getUtils({ - pageIsDynamic, - page: matchedPathnameNoExt, - i18n: this.nextConfig.i18n, - basePath: this.nextConfig.basePath, - rewrites: combinedRewrites, - }) + combinedRewrites.push(...this.customRoutes.rewrites.beforeFiles) + combinedRewrites.push(...this.customRoutes.rewrites.afterFiles) + combinedRewrites.push(...this.customRoutes.rewrites.fallback) - utils.handleRewrites(req, parsedUrl) + const utils = getUtils({ + pageIsDynamic, + page: matchedPathnameNoExt, + i18n: this.nextConfig.i18n, + basePath: this.nextConfig.basePath, + rewrites: combinedRewrites, + }) - // interpolate dynamic params and normalize URL if needed - if (pageIsDynamic) { - let params: ParsedUrlQuery | false = {} + utils.handleRewrites(req, parsedUrl) - Object.assign(parsedUrl.query, query) - const paramsResult = utils.normalizeDynamicRouteParams(parsedUrl.query) + // interpolate dynamic params and normalize URL if needed + if (pageIsDynamic) { + let params: ParsedUrlQuery | false = {} - if (paramsResult.hasValidParams) { - params = paramsResult.params - } else if (req.headers['x-now-route-matches']) { - const opts: Record = {} - params = utils.getParamsFromRouteMatches( - req, - opts, - (parsedUrl.query.__nextLocale as string | undefined) || '' + Object.assign(parsedUrl.query, query) + const paramsResult = utils.normalizeDynamicRouteParams( + parsedUrl.query ) - if (opts.locale) { - parsedUrl.query.__nextLocale = opts.locale + if (paramsResult.hasValidParams) { + params = paramsResult.params + } else if (req.headers['x-now-route-matches']) { + const opts: Record = {} + params = utils.getParamsFromRouteMatches( + req, + opts, + (parsedUrl.query.__nextLocale as string | undefined) || '' + ) + + if (opts.locale) { + parsedUrl.query.__nextLocale = opts.locale + } + } else { + params = utils.dynamicRouteMatcher!(matchedPathnameNoExt) } - } else { - params = utils.dynamicRouteMatcher!(matchedPathnameNoExt) - } - if (params) { - params = utils.normalizeDynamicRouteParams(params).params + if (params) { + params = utils.normalizeDynamicRouteParams(params).params - matchedPathname = utils.interpolateDynamicPath( - matchedPathname, - params - ) - req.url = utils.interpolateDynamicPath(req.url!, params) - } + matchedPathname = utils.interpolateDynamicPath( + matchedPathname, + params + ) + req.url = utils.interpolateDynamicPath(req.url!, params) + } - if (reqUrlIsDataUrl && matchedPathIsDataUrl) { - req.url = formatUrl({ - ...parsedPath, - pathname: matchedPathname, - }) + if (reqUrlIsDataUrl && matchedPathIsDataUrl) { + req.url = formatUrl({ + ...parsedPath, + pathname: matchedPathname, + }) + } + + Object.assign(parsedUrl.query, params) + utils.normalizeVercelUrl(req, true) } - Object.assign(parsedUrl.query, params) - utils.normalizeVercelUrl(req, true) + parsedUrl.pathname = `${basePath || ''}${ + matchedPathname === '/' && basePath ? '' : matchedPathname + }` } - parsedUrl.pathname = `${basePath || ''}${ - matchedPathname === '/' && basePath ? '' : matchedPathname - }` - } + ;(req as any).__nextHadTrailingSlash = url.locale?.trailingSlash + if (url.locale?.domain) { + ;(req as any).__nextIsLocaleDomain = true + } - ;(req as any).__nextHadTrailingSlash = url.locale?.trailingSlash - if (url.locale?.domain) { - ;(req as any).__nextIsLocaleDomain = true - } + if (url.locale?.path.detectedLocale) { + req.url = formatUrl(url) + ;(req as any).__nextStrippedLocale = true + if (url.pathname === '/api' || url.pathname.startsWith('/api/')) { + return this.render404(req, res, parsedUrl) + } + } - if (url.locale?.path.detectedLocale) { - req.url = formatUrl(url) - ;(req as any).__nextStrippedLocale = true - if (url.pathname === '/api' || url.pathname.startsWith('/api/')) { - return this.render404(req, res, parsedUrl) + if (!this.minimalMode || !parsedUrl.query.__nextLocale) { + if (url?.locale?.locale) { + parsedUrl.query.__nextLocale = url.locale.locale + } } - } - if (!this.minimalMode || !parsedUrl.query.__nextLocale) { - if (url?.locale?.locale) { - parsedUrl.query.__nextLocale = url.locale.locale + if (url?.locale?.defaultLocale) { + parsedUrl.query.__nextDefaultLocale = url.locale.defaultLocale } - } - if (url?.locale?.defaultLocale) { - parsedUrl.query.__nextDefaultLocale = url.locale.defaultLocale - } + if (url.locale?.redirect) { + res.setHeader('Location', url.locale.redirect) + res.statusCode = TEMPORARY_REDIRECT_STATUS + res.end() + return + } - if (url.locale?.redirect) { - res.setHeader('Location', url.locale.redirect) - res.statusCode = TEMPORARY_REDIRECT_STATUS - res.end() - return - } + res.statusCode = 200 - res.statusCode = 200 - try { return await this.run(req, res, parsedUrl) } catch (err) { + if ( + (err && typeof err === 'object' && err.code === 'ERR_INVALID_URL') || + err instanceof DecodeError + ) { + res.statusCode = 400 + return this.renderError(null, req, res, '/_error', {}) + } + if (this.minimalMode || this.renderOpts.dev) { throw err } diff --git a/test/integration/production/test/security.js b/test/integration/production/test/security.js index fd674399249bf57..f9a85ea09b36c37 100644 --- a/test/integration/production/test/security.js +++ b/test/integration/production/test/security.js @@ -27,6 +27,32 @@ async function checkInjected(browser) { module.exports = (context) => { describe('With Security Related Issues', () => { + it('should handle invalid URL properly', async () => { + async function invalidRequest() { + return new Promise((resolve, reject) => { + const request = http.request( + { + hostname: `localhost`, + port: context.appPort, + path: `*`, + }, + (response) => { + resolve(response.statusCode) + } + ) + request.on('error', (err) => reject(err)) + request.end() + }) + } + try { + expect(await invalidRequest()).toBe(400) + expect(await invalidRequest()).toBe(400) + } catch (err) { + // eslint-disable-next-line + expect(err.code).toBe('ECONNREFUSED') + } + }) + it('should only access files inside .next directory', async () => { const buildId = readFileSync(join(__dirname, '../.next/BUILD_ID'), 'utf8')