From 604633de47747ed04cb417b7a18505d52192d9d9 Mon Sep 17 00:00:00 2001 From: "alexander.akait" Date: Wed, 20 Mar 2024 18:14:37 +0300 Subject: [PATCH] fix(security): do not allow to read files above --- src/middleware.js | 16 +++- src/utils/compatibleAPI.js | 116 ++++++++++++++++++++++++++++ src/utils/getFilenameFromUrl.js | 97 +++++++++++++++++------ types/utils/compatibleAPI.d.ts | 12 +++ types/utils/getFilenameFromUrl.d.ts | 10 ++- 5 files changed, 225 insertions(+), 26 deletions(-) diff --git a/src/middleware.js b/src/middleware.js index 1ed11c5e1..b44c1bef4 100644 --- a/src/middleware.js +++ b/src/middleware.js @@ -11,6 +11,7 @@ const { setHeaderForResponse, setStatusCode, send, + sendError, } = require("./utils/compatibleAPI"); const ready = require("./utils/ready"); @@ -95,9 +96,12 @@ function wrapper(context) { } async function processRequest() { + /** @type {import("./utils/getFilenameFromUrl").Extra} */ + const extra = {}; const filename = getFilenameFromUrl( context, - /** @type {string} */ (req.url) + /** @type {string} */ (req.url), + extra ); if (!filename) { @@ -106,6 +110,16 @@ function wrapper(context) { return; } + if (extra.errorCode) { + if (extra.errorCode === 403) { + context.logger.error(`Malicious path "${filename}".`); + } + + sendError(req, res, extra.errorCode); + + return; + } + let { headers } = context.options; if (typeof headers === "function") { diff --git a/src/utils/compatibleAPI.js b/src/utils/compatibleAPI.js index bd094daa3..f39c7287a 100644 --- a/src/utils/compatibleAPI.js +++ b/src/utils/compatibleAPI.js @@ -155,6 +155,121 @@ function send(req, res, bufferOtStream, byteLength) { } } +/** + * @template {ServerResponse} Response + * @param {Response} res + */ +function clearHeadersForResponse(res) { + const headers = getHeaderNames(res); + + for (let i = 0; i < headers.length; i++) { + res.removeHeader(headers[i]); + } +} + +const matchHtmlRegExp = /["'&<>]/; + +/** + * @param {string} string raw HTML + * @returns {string} escaped HTML + */ +function escapeHtml(string) { + const str = `${string}`; + const match = matchHtmlRegExp.exec(str); + + if (!match) { + return str; + } + + let escape; + let html = ""; + let index = 0; + let lastIndex = 0; + + for ({ index } = match; index < str.length; index++) { + switch (str.charCodeAt(index)) { + // " + case 34: + escape = """; + break; + // & + case 38: + escape = "&"; + break; + // ' + case 39: + escape = "'"; + break; + // < + case 60: + escape = "<"; + break; + // > + case 62: + escape = ">"; + break; + default: + // eslint-disable-next-line no-continue + continue; + } + + if (lastIndex !== index) { + html += str.substring(lastIndex, index); + } + + lastIndex = index + 1; + html += escape; + } + + return lastIndex !== index ? html + str.substring(lastIndex, index) : html; +} + +/** @type {Record} */ +const statuses = { + 400: "Bad Request", + 403: "Forbidden", + 404: "Not Found", + 416: "Range Not Satisfiable", + 500: "Internal Server Error", +}; + +/** + * @template {IncomingMessage} Request + * @template {ServerResponse} Response + * @param {Request} req response + * @param {Response} res response + * @param {number} status status + * @returns {void} + */ +function sendError(req, res, status) { + const content = statuses[status] || String(status); + const document = ` + + + +Error + + +
${escapeHtml(content)}
+ +`; + + // Clear existing headers + clearHeadersForResponse(res); + + // Send basic response + setStatusCode(res, status); + setHeaderForResponse(res, "Content-Type", "text/html; charset=utf-8"); + setHeaderForResponse(res, "Content-Security-Policy", "default-src 'none'"); + setHeaderForResponse(res, "X-Content-Type-Options", "nosniff"); + + const byteLength = Buffer.byteLength(document); + + setHeaderForResponse(res, "Content-Length", byteLength); + + res.end(document); +} + module.exports = { getHeaderNames, getHeaderFromRequest, @@ -162,4 +277,5 @@ module.exports = { setHeaderForResponse, setStatusCode, send, + sendError, }; diff --git a/src/utils/getFilenameFromUrl.js b/src/utils/getFilenameFromUrl.js index 4aa845b9a..25720e1dc 100644 --- a/src/utils/getFilenameFromUrl.js +++ b/src/utils/getFilenameFromUrl.js @@ -10,11 +10,14 @@ const getPaths = require("./getPaths"); const cacheStore = new WeakMap(); /** + * @template T * @param {Function} fn - * @param {{ cache?: Map }} [cache] + * @param {{ cache?: Map } | undefined} cache + * @param {(value: T) => T} callback * @returns {any} */ -const mem = (fn, { cache = new Map() } = {}) => { +// @ts-ignore +const mem = (fn, { cache = new Map() } = {}, callback) => { /** * @param {any} arguments_ * @return {any} @@ -27,7 +30,8 @@ const mem = (fn, { cache = new Map() } = {}) => { return cacheItem.data; } - const result = fn.apply(this, arguments_); + let result = fn.apply(this, arguments_); + result = callback(result); cache.set(key, { data: result, @@ -40,20 +44,52 @@ const mem = (fn, { cache = new Map() } = {}) => { return memoized; }; -const memoizedParse = mem(parse); +// eslint-disable-next-line no-undefined +const memoizedParse = mem(parse, undefined, (value) => { + if (value.pathname) { + // eslint-disable-next-line no-param-reassign + value.pathname = decode(value.pathname); + } + + return value; +}); + +const UP_PATH_REGEXP = /(?:^|[\\/])\.\.(?:[\\/]|$)/; + +/** + * @typedef {Object} Extra + * @property {import("fs").Stats=} stats + * @property {number=} errorCode + */ + +/** + * decodeURIComponent. + * + * Allows V8 to only deoptimize this fn instead of all of send(). + * + * @param {string} input + * @returns {string} + */ + +function decode(input) { + return querystring.unescape(input); +} /** * @template {IncomingMessage} Request * @template {ServerResponse} Response * @param {import("../index.js").Context} context * @param {string} url + * @param {Extra=} extra * @returns {string | undefined} */ -function getFilenameFromUrl(context, url) { +function getFilenameFromUrl(context, url, extra = {}) { const { options } = context; const paths = getPaths(context); + /** @type {string | undefined} */ let foundFilename; + /** @type {URL} */ let urlObject; try { @@ -64,7 +100,9 @@ function getFilenameFromUrl(context, url) { } for (const { publicPath, outputPath } of paths) { + /** @type {string | undefined} */ let filename; + /** @type {URL} */ let publicPathObject; try { @@ -78,26 +116,38 @@ function getFilenameFromUrl(context, url) { continue; } - if ( - urlObject.pathname && - urlObject.pathname.startsWith(publicPathObject.pathname) - ) { - filename = outputPath; + const { pathname } = urlObject; + const { pathname: publicPathPathname } = publicPathObject; - // Strip the `pathname` property from the `publicPath` option from the start of requested url - // `/complex/foo.js` => `foo.js` - const pathname = urlObject.pathname.slice( - publicPathObject.pathname.length - ); + if (pathname && pathname.startsWith(publicPathPathname)) { + // Null byte(s) + if (pathname.includes("\0")) { + // eslint-disable-next-line no-param-reassign + extra.errorCode = 400; + + return; + } + + // ".." is malicious + if (UP_PATH_REGEXP.test(path.normalize(`./${pathname}`))) { + // eslint-disable-next-line no-param-reassign + extra.errorCode = 403; - if (pathname) { - filename = path.join(outputPath, querystring.unescape(pathname)); + return; } - let fsStats; + // Strip the `pathname` property from the `publicPath` option from the start of requested url + // `/complex/foo.js` => `foo.js` + // and add outputPath + // `foo.js` => `/home/user/my-project/dist/foo.js` + filename = path.join( + outputPath, + pathname.slice(publicPathPathname.length) + ); try { - fsStats = + // eslint-disable-next-line no-param-reassign + extra.stats = /** @type {import("fs").statSync} */ (context.outputFileSystem.statSync)(filename); } catch (_ignoreError) { @@ -105,12 +155,12 @@ function getFilenameFromUrl(context, url) { continue; } - if (fsStats.isFile()) { + if (extra.stats.isFile()) { foundFilename = filename; break; } else if ( - fsStats.isDirectory() && + extra.stats.isDirectory() && (typeof options.index === "undefined" || options.index) ) { const indexValue = @@ -122,7 +172,8 @@ function getFilenameFromUrl(context, url) { filename = path.join(filename, indexValue); try { - fsStats = + // eslint-disable-next-line no-param-reassign + extra.stats = /** @type {import("fs").statSync} */ (context.outputFileSystem.statSync)(filename); } catch (__ignoreError) { @@ -130,7 +181,7 @@ function getFilenameFromUrl(context, url) { continue; } - if (fsStats.isFile()) { + if (extra.stats.isFile()) { foundFilename = filename; break; diff --git a/types/utils/compatibleAPI.d.ts b/types/utils/compatibleAPI.d.ts index 833fed359..79df9659d 100644 --- a/types/utils/compatibleAPI.d.ts +++ b/types/utils/compatibleAPI.d.ts @@ -84,3 +84,15 @@ export function send< bufferOtStream: string | Buffer | import("fs").ReadStream, byteLength: number ): void; +/** + * @template {IncomingMessage} Request + * @template {ServerResponse} Response + * @param {Request} req response + * @param {Response} res response + * @param {number} status status + * @returns {void} + */ +export function sendError< + Request_1 extends import("http").IncomingMessage, + Response_1 extends import("../index.js").ServerResponse +>(req: Request_1, res: Response_1, status: number): void; diff --git a/types/utils/getFilenameFromUrl.d.ts b/types/utils/getFilenameFromUrl.d.ts index 8ad2ff148..9d0b205b3 100644 --- a/types/utils/getFilenameFromUrl.d.ts +++ b/types/utils/getFilenameFromUrl.d.ts @@ -5,6 +5,7 @@ export = getFilenameFromUrl; * @template {ServerResponse} Response * @param {import("../index.js").Context} context * @param {string} url + * @param {Extra=} extra * @returns {string | undefined} */ declare function getFilenameFromUrl< @@ -12,10 +13,15 @@ declare function getFilenameFromUrl< Response_1 extends import("../index.js").ServerResponse >( context: import("../index.js").Context, - url: string + url: string, + extra?: Extra | undefined ): string | undefined; declare namespace getFilenameFromUrl { - export { IncomingMessage, ServerResponse }; + export { Extra, IncomingMessage, ServerResponse }; } +type Extra = { + stats?: import("fs").Stats | undefined; + errorCode?: number | undefined; +}; type IncomingMessage = import("../index.js").IncomingMessage; type ServerResponse = import("../index.js").ServerResponse;