Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix(security): do not allow to read files above #1779

Merged
merged 1 commit into from Mar 20, 2024
Merged

fix(security): do not allow to read files above #1779

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 15 additions & 1 deletion src/middleware.js
View file
Open in desktop
Expand Up @@ -11,6 +11,7 @@ const {
setHeaderForResponse,
setStatusCode,
send,
sendError,
} = require("./utils/compatibleAPI");
const ready = require("./utils/ready");

Expand Down Expand Up @@ -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) {
Expand All @@ -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") {
Expand Down
116 changes: 116 additions & 0 deletions src/utils/compatibleAPI.js
View file
Open in desktop
Expand Up @@ -155,11 +155,127 @@ 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 = "&quot;";
break;
// &
case 38:
escape = "&amp;";
break;
// '
case 39:
escape = "&#39;";
break;
// <
case 60:
escape = "&lt;";
break;
// >
case 62:
escape = "&gt;";
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<number, string>} */
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 = `<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8">
<title>Error</title>
</head>
<body>
<pre>${escapeHtml(content)}</pre>
</body>
</html>`;

// 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,
getHeaderFromResponse,
setHeaderForResponse,
setStatusCode,
send,
sendError,
};
97 changes: 74 additions & 23 deletions src/utils/getFilenameFromUrl.js
View file
Open in desktop
Expand Up @@ -10,11 +10,14 @@ const getPaths = require("./getPaths");
const cacheStore = new WeakMap();

/**
* @template T
* @param {Function} fn
* @param {{ cache?: Map<any, any> }} [cache]
* @param {{ cache?: Map<string, { data: T }> } | 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}
Expand All @@ -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,
Expand All @@ -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<Request, Response>} 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 {
Expand All @@ -64,7 +100,9 @@ function getFilenameFromUrl(context, url) {
}

for (const { publicPath, outputPath } of paths) {
/** @type {string | undefined} */
let filename;
/** @type {URL} */
let publicPathObject;

try {
Expand All @@ -78,39 +116,51 @@ 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;
Comment on lines +131 to +134
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the key change, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes


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) {
// eslint-disable-next-line no-continue
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 =
Expand All @@ -122,15 +172,16 @@ 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) {
// eslint-disable-next-line no-continue
continue;
}

if (fsStats.isFile()) {
if (extra.stats.isFile()) {
foundFilename = filename;

break;
Expand Down
12 changes: 12 additions & 0 deletions types/utils/compatibleAPI.d.ts
View file
Open in desktop
Expand Up @@ -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;
10 changes: 8 additions & 2 deletions types/utils/getFilenameFromUrl.d.ts
View file
Open in desktop
Expand Up @@ -5,17 +5,23 @@ export = getFilenameFromUrl;
* @template {ServerResponse} Response
* @param {import("../index.js").Context<Request, Response>} context
* @param {string} url
* @param {Extra=} extra
* @returns {string | undefined}
*/
declare function getFilenameFromUrl<
Request_1 extends import("http").IncomingMessage,
Response_1 extends import("../index.js").ServerResponse
>(
context: import("../index.js").Context<Request_1, Response_1>,
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;