From 383dad7e20788d5c1edd8901ab9d532e6ea6691b Mon Sep 17 00:00:00 2001 From: Thomas Durieux <5577568+tdurieux@users.noreply.github.com> Date: Tue, 15 Sep 2026 20:26:42 +0000 Subject: [PATCH] fix: prevent path traversal through hashed asset routes --- src/server/index.ts | 16 ++++-- test/hashed-assets.test.js | 105 +++++++++++++++++++++++++++++++++++++ 2 files changed, 118 insertions(+), 3 deletions(-) create mode 100644 test/hashed-assets.test.js diff --git a/src/server/index.ts b/src/server/index.ts index fd7639e..eac73e1 100644 --- a/src/server/index.ts +++ b/src/server/index.ts @@ -3,7 +3,7 @@ import { config as dotenv } from "dotenv"; dotenv(); import { createClient } from "redis"; -import { resolve, join } from "path"; +import { resolve, join, sep } from "path"; import { existsSync, readFileSync } from "fs"; import rateLimit from "express-rate-limit"; import { slowDown } from "express-slow-down"; @@ -338,10 +338,20 @@ export default async function start() { const dir = req.params[0]; // "script" or "css" const base = req.params[1]; // e.g. "core" const ext = req.params[3]; // e.g. "min.js" - const filePath = join("public", dir, `${base}.${ext}`); + // Express decodes captures, so validate before resolving the filename. + if ( + (dir !== "script" && dir !== "css") || + base.startsWith(".") || + /[/\\\0]/.test(base) + ) { + return res.status(404).end(); + } + const assetRoot = resolve("public", dir); + const filePath = resolve(assetRoot, `${base}.${ext}`); + if (!filePath.startsWith(assetRoot + sep)) return res.status(404).end(); if (!existsSync(filePath)) return next(); res.set("Cache-Control", "public, max-age=31536000, immutable"); - res.sendFile(resolve(filePath)); + res.sendFile(filePath); } ); diff --git a/test/hashed-assets.test.js b/test/hashed-assets.test.js new file mode 100644 index 0000000..9b9be9c --- /dev/null +++ b/test/hashed-assets.test.js @@ -0,0 +1,105 @@ +const { expect } = require("chai"); +const express = require("express"); +const fs = require("node:fs"); +const http = require("node:http"); +const os = require("node:os"); +const path = require("node:path"); +const ts = require("typescript"); + +function request(server, url) { + return new Promise((resolve, reject) => { + http.get({ host: "127.0.0.1", port: server.address().port, path: url }, res => { + let body = ""; + res.setEncoding("utf8"); + res.on("data", chunk => { body += chunk; }); + res.on("end", () => resolve({ status: res.statusCode, headers: res.headers, body })); + }).on("error", reject); + }); +} + +describe("hashed asset route", function () { + let directory; + let server; + + before(async function () { + directory = fs.mkdtempSync(path.join(os.tmpdir(), "hashed-assets-")); + for (const [name, content] of Object.entries({ + "public/script/core.min.js": "window.core = true;", + "public/script/plain.js": "window.plain = true;", + "public/css/all.min.css": "body { color: red; }", + "private.json": "private fixture outside public", + "public-other/private.json": "private fixture in sibling directory", + })) { + const target = path.join(directory, name); + fs.mkdirSync(path.dirname(target), { recursive: true }); + fs.writeFileSync(target, content); + } + + // Execute the production route without starting its databases and workers. + const source = fs.readFileSync(path.join(__dirname, "../src/server/index.ts"), "utf8"); + const start = source.indexOf(" // Hashed assets"); + const end = source.indexOf("\n app.use(", start); + expect(start).to.be.greaterThan(-1); + expect(end).to.be.greaterThan(start); + const app = express(); + const context = { + app, + join: path.join, + sep: path.sep, + resolve: (...parts) => path.resolve(directory, ...parts), + existsSync: file => fs.existsSync(path.resolve(directory, file)), + }; + new Function(...Object.keys(context), ts.transpileModule(source.slice(start, end), {}).outputText)(...Object.values(context)); + app.use(express.static(path.join(directory, "public"), { index: false })); + app.use((_req, res) => res.status(404).end()); + server = await new Promise(resolve => { + const listener = app.listen(0, "127.0.0.1", () => resolve(listener)); + }); + }); + + after(async function () { + if (server) await new Promise(resolve => server.close(resolve)); + if (directory) fs.rmSync(directory, { recursive: true, force: true }); + }); + + for (const [url, content] of [ + ["/script/core.aaaaaaaaaa.min.js", "window.core = true;"], + ["/script/plain.aaaaaaaaaa.js", "window.plain = true;"], + ["/css/all.aaaaaaaaaa.min.css", "body { color: red; }"], + ]) { + it(`serves ${url} with immutable caching`, async function () { + const response = await request(server, url); + expect(response.status).to.equal(200); + expect(response.body).to.equal(content); + expect(response.headers["cache-control"]).to.equal("public, max-age=31536000, immutable"); + }); + } + + for (const dir of ["script", "css"]) { + for (const base of [ + "../../private", + "..%2f..%2fprivate", + "%2e%2e%2f%2e%2e%2fprivate", + "..%2F..%2Fprivate", + "..%5c..%5cprivate", + "../../public-other/private", + ]) { + it(`rejects traversal through /${dir}/${base}`, async function () { + const response = await request(server, `/${dir}/${base}.aaaaaaaaaa.json`); + expect(response.status).to.equal(404); + expect(response.body).not.to.include("private fixture"); + expect(response.headers["cache-control"]).to.equal(undefined); + }); + } + } + + it("falls through for a missing hashed asset", async function () { + expect((await request(server, "/script/missing.aaaaaaaaaa.js")).status).to.equal(404); + }); + + it("still serves unhashed static assets", async function () { + const response = await request(server, "/script/plain.js"); + expect(response.status).to.equal(200); + expect(response.body).to.equal("window.plain = true;"); + }); +});