Skip to content

Commit cefccba

Browse files
authored
Merge commit from fork
* test: cover path traversal when publicPath has no trailing slash Ref: GHSA-g84c-rxfj-3j2c * fix: reject requests that resolve outside the output directory Ref: GHSA-g84c-rxfj-3j2c
1 parent ec97e06 commit cefccba

2 files changed

Lines changed: 74 additions & 0 deletions

File tree

‎src/utils/getFilenameFromUrl.js‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,24 @@ function getFilenameFromUrl(context, url, extra = {}) {
115115
pathname.slice(publicPathPathname.length),
116116
);
117117

118+
// The resolved filename must stay within `outputPath`. When `publicPath`
119+
// has no trailing slash a sibling path can share its prefix (for example
120+
// `/assets../secret` starts with `/assets`), so the stripped remainder may
121+
// contain `..` that escapes the output root once joined. The `..` guard
122+
// above runs on the un-stripped pathname and does not catch this, so
123+
// verify containment on the final path.
124+
const relative = path.relative(outputPath, filename);
125+
126+
if (
127+
relative === ".." ||
128+
relative.startsWith(`..${path.sep}`) ||
129+
path.isAbsolute(relative)
130+
) {
131+
extra.errorCode = 403;
132+
133+
return;
134+
}
135+
118136
try {
119137
extra.stats = context.outputFileSystem.statSync(filename);
120138
} catch {

‎test/middleware.test.js‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -833,6 +833,62 @@ describe.each([
833833
});
834834

835835
describe("basic", () => {
836+
// @see https://github.com/webpack/webpack-dev-middleware/security/advisories/GHSA-g84c-rxfj-3j2c
837+
describe("should not allow path traversal when publicPath has no trailing slash", () => {
838+
let compiler;
839+
840+
const outputPath = path.resolve(
841+
__dirname,
842+
"./outputs/traversal-no-slash-public-path",
843+
);
844+
845+
beforeAll(async () => {
846+
compiler = getCompiler({
847+
...webpackConfig,
848+
output: {
849+
filename: "bundle.js",
850+
path: outputPath,
851+
publicPath: "/static",
852+
},
853+
});
854+
855+
[server, req, instance] = await frameworkFactory(
856+
name,
857+
framework,
858+
compiler,
859+
);
860+
861+
instance.context.outputFileSystem.mkdirSync(outputPath, {
862+
recursive: true,
863+
});
864+
instance.context.outputFileSystem.writeFileSync(
865+
path.resolve(outputPath, "index.html"),
866+
"HTML",
867+
);
868+
// A sensitive file placed one level above the served output root.
869+
instance.context.outputFileSystem.writeFileSync(
870+
path.resolve(outputPath, "../secret.txt"),
871+
"TOP SECRET",
872+
);
873+
});
874+
875+
afterAll(async () => {
876+
await close(server, instance);
877+
});
878+
879+
it('should serve a legitimate file under the "/static" prefix', async () => {
880+
const response = await req.get("/static/index.html");
881+
882+
expect(response.statusCode).toBe(200);
883+
});
884+
885+
it("should not allow to get files above root", async () => {
886+
const response = await req.get("/static../secret.txt");
887+
888+
expect(response.statusCode).toBe(403);
889+
});
890+
});
891+
836892
describe("should work", () => {
837893
let compiler;
838894
let codeContent;

0 commit comments

Comments
 (0)