mirror of
https://github.com/tdurieux/anonymous_github.git
synced 2026-09-14 14:48:59 +02:00
fix: decode and validate webview file paths
This commit is contained in:
@@ -83,13 +83,19 @@ async function webView(req: express.Request, res: express.Response) {
|
|||||||
const wRoot = repo.options.pageSource.path;
|
const wRoot = repo.options.pageSource.path;
|
||||||
|
|
||||||
const indexRepoId = req.path.indexOf(req.params.repoId);
|
const indexRepoId = req.path.indexOf(req.params.repoId);
|
||||||
const filePath = req.path.substring(
|
const rawPath = req.path.substring(
|
||||||
indexRepoId + req.params.repoId.length + 1
|
indexRepoId + req.params.repoId.length + 1
|
||||||
);
|
);
|
||||||
|
let filePath: string;
|
||||||
|
try {
|
||||||
|
filePath = decodeURIComponent(rawPath);
|
||||||
|
} catch {
|
||||||
|
throw new AnonymousError("invalid_path", { httpStatus: 400 });
|
||||||
|
}
|
||||||
// Reject traversal in the URL-derived segment before joining it onto the
|
// Reject traversal in the URL-derived segment before joining it onto the
|
||||||
// page-source root. Stripping a single leading "/" or "." is not enough
|
// page-source root. Stripping a single leading "/" or "." is not enough
|
||||||
// to stop "../../" sequences from climbing out of the repo (CWE-22).
|
// to stop "../../" sequences from climbing out of the repo (CWE-22).
|
||||||
if (filePath.split(/[\\/]/).some((segment) => segment === "..")) {
|
if (/^[\\/]/.test(filePath) || filePath.split(/[\\/]/).some((segment) => segment === "..")) {
|
||||||
throw new AnonymousError("invalid_path", {
|
throw new AnonymousError("invalid_path", {
|
||||||
httpStatus: 400,
|
httpStatus: 400,
|
||||||
object: filePath,
|
object: filePath,
|
||||||
|
|||||||
@@ -152,7 +152,7 @@ describe("production regressions", function () {
|
|||||||
expect(second.headers.ETag).not.to.equal(first.headers.ETag);
|
expect(second.headers.ETag).not.to.equal(first.headers.ETag);
|
||||||
expect(second.statusCode).not.to.equal(304);
|
expect(second.statusCode).not.to.equal(304);
|
||||||
});
|
});
|
||||||
it("sandboxes rendered webview documents", async function () {
|
it("decodes webview filenames and sandboxes rendered documents", async function () {
|
||||||
const File = require("../src/core/AnonymizedFile").default;
|
const File = require("../src/core/AnonymizedFile").default;
|
||||||
const utils = require("../src/server/routes/route-utils");
|
const utils = require("../src/server/routes/route-utils");
|
||||||
const repo = { options: { terms: [], page: true, pageSource: { path: "/", branch: "main" }, image: true }, model: { source: { branch: "main" } } };
|
const repo = { options: { terms: [], page: true, pageSource: { path: "/", branch: "main" }, image: true }, model: { source: { branch: "main" } } };
|
||||||
@@ -162,7 +162,7 @@ describe("production regressions", function () {
|
|||||||
stub(File.prototype, "send", async () => {});
|
stub(File.prototype, "send", async () => {});
|
||||||
const handler = require("../src/server/routes/webview").default.stack[0].route.stack[0].handle;
|
const handler = require("../src/server/routes/webview").default.stack[0].route.stack[0].handle;
|
||||||
const res = response();
|
const res = response();
|
||||||
await handler({ path: "/repo/my file.html", params: { repoId: "repo" }, headers: {} }, res);
|
await handler({ path: "/repo/my%20file.html", params: { repoId: "repo" }, headers: {} }, res);
|
||||||
expect(path).to.equal("my file.html");
|
expect(path).to.equal("my file.html");
|
||||||
expect(res.headers["Content-Security-Policy"]).to.include("sandbox");
|
expect(res.headers["Content-Security-Policy"]).to.include("sandbox");
|
||||||
expect(res.headers["Content-Security-Policy"]).not.to.include("allow-same-origin");
|
expect(res.headers["Content-Security-Policy"]).not.to.include("allow-same-origin");
|
||||||
|
|||||||
Reference in New Issue
Block a user