Guard meeting :id against path traversal
saveMeeting/loadMeeting/deleteMeeting built path.join(meetingsDir, id + '.json') straight from req.params.id, so an admin-authed :id like '../../etc/passwd' could read/write/delete outside internal-meetings/. Centralize a meetingPath() helper that strips anything outside [A-Za-z0-9_-] (mirrors output-store.js) and throws on an empty result; load/delete catch it as 404/no-op. Add a regression test.
This commit is contained in:
@@ -77,19 +77,31 @@ async function ensureMeetingsDir(dataDir) {
|
||||
await fs.mkdir(meetingsDir(dataDir), { recursive: true }).catch(() => {});
|
||||
}
|
||||
|
||||
// Build the on-disk path for a meeting record, sanitizing the id so a
|
||||
// caller-supplied :id can't traverse out of internal-meetings/. Real
|
||||
// ids are UUIDs; anything outside [A-Za-z0-9_-] is stripped (mirrors
|
||||
// output-store.js's pathFor). Throws when the id sanitizes to empty —
|
||||
// load/delete catch it (→ 404 / no-op); save only ever gets a freshly
|
||||
// minted id.
|
||||
export function meetingPath(dataDir, id) {
|
||||
const safe = String(id || "").replace(/[^A-Za-z0-9_-]/g, "");
|
||||
if (!safe) throw new Error("invalid meeting id");
|
||||
return path.join(meetingsDir(dataDir), `${safe}.json`);
|
||||
}
|
||||
|
||||
// ─── Storage layer ──────────────────────────────────────────────────
|
||||
|
||||
async function saveMeeting(dataDir, id, record) {
|
||||
await ensureMeetingsDir(dataDir);
|
||||
const filePath = path.join(meetingsDir(dataDir), `${id}.json`);
|
||||
const filePath = meetingPath(dataDir, id);
|
||||
await fs.writeFile(filePath, JSON.stringify(record, null, 2), {
|
||||
mode: 0o600,
|
||||
});
|
||||
}
|
||||
|
||||
async function loadMeeting(dataDir, id) {
|
||||
const filePath = path.join(meetingsDir(dataDir), `${id}.json`);
|
||||
try {
|
||||
const filePath = meetingPath(dataDir, id);
|
||||
const raw = await fs.readFile(filePath, "utf8");
|
||||
const rec = JSON.parse(raw);
|
||||
// Retroactive chunk-contiguity backfill must run BEFORE the
|
||||
@@ -239,8 +251,8 @@ async function listMeetings(dataDir) {
|
||||
}
|
||||
|
||||
async function deleteMeeting(dataDir, id) {
|
||||
const filePath = path.join(meetingsDir(dataDir), `${id}.json`);
|
||||
try {
|
||||
const filePath = meetingPath(dataDir, id);
|
||||
await fs.unlink(filePath);
|
||||
return true;
|
||||
} catch {
|
||||
|
||||
@@ -0,0 +1,34 @@
|
||||
// Path-traversal guard for meeting record ids (internal-meetings.js
|
||||
// meetingPath). A caller-supplied :id must never escape the
|
||||
// internal-meetings/ directory.
|
||||
|
||||
import { test, describe } from "node:test";
|
||||
import assert from "node:assert/strict";
|
||||
import path from "node:path";
|
||||
import { meetingPath } from "../routes/internal-meetings.js";
|
||||
|
||||
const DATA = "/data";
|
||||
const DIR = path.join(DATA, "internal-meetings");
|
||||
|
||||
describe("meetingPath", () => {
|
||||
test("a normal UUID id maps into internal-meetings/", () => {
|
||||
const id = "2f1c9b3a-0e4d-4a77-9d2a-abc123def456";
|
||||
assert.equal(meetingPath(DATA, id), path.join(DIR, `${id}.json`));
|
||||
});
|
||||
|
||||
test("traversal-shaped ids are sanitized and stay inside the dir", () => {
|
||||
for (const id of ["../../etc/passwd", "../../../root/.ssh/id", "..%2f..%2fx", "a/b/c", "....//x"]) {
|
||||
const p = meetingPath(DATA, id);
|
||||
const rel = path.relative(DIR, p);
|
||||
assert.ok(!rel.startsWith(".."), `${id} escaped to ${p}`);
|
||||
assert.ok(!p.includes(".."), `${id} left ".." in ${p}`);
|
||||
assert.ok(p.endsWith(".json"));
|
||||
}
|
||||
});
|
||||
|
||||
test("an id that sanitizes to empty throws (load/delete catch → 404 / no-op)", () => {
|
||||
for (const id of ["", null, undefined, "/", "../", "...", "!!!"]) {
|
||||
assert.throws(() => meetingPath(DATA, id), /invalid meeting id/);
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user