From d112ef9a1bb282243852381f3d64bca136a0fcff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emrik=20=C3=96stling?= Date: Wed, 9 Sep 2026 22:47:13 +0200 Subject: [PATCH] fix symlink in downloads --- src/helpers/env.ts | 3 ++ src/index.tsx | 4 +- src/native/landlock-runner.c | 31 +++++++++--- src/pages/download.tsx | 41 ++++++++++++---- tests/db/db.test.ts | 9 ++-- tests/helpers/sandbox.test.ts | 14 +++++- tests/pages/download.test.ts | 90 +++++++++++++++++++++++++++++++++++ 7 files changed, 166 insertions(+), 26 deletions(-) create mode 100644 tests/pages/download.test.ts diff --git a/src/helpers/env.ts b/src/helpers/env.ts index 53f6a8fc..68d511b1 100644 --- a/src/helpers/env.ts +++ b/src/helpers/env.ts @@ -25,3 +25,6 @@ export const UNAUTHENTICATED_USER_SHARING = process.env.UNAUTHENTICATED_USER_SHARING?.toLowerCase() === "true" || false; export const TIMEZONE = process.env.TZ || undefined; + +export const uploadsDir = "./data/uploads/"; +export const outputDir = "./data/output/"; diff --git a/src/index.tsx b/src/index.tsx index 2ce61c80..21605567 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -4,7 +4,7 @@ import { Elysia } from "elysia"; import { rmSync } from "node:fs"; import db from "./db/db"; import { Jobs } from "./db/types"; -import { AUTO_DELETE_EVERY_N_HOURS, WEBROOT } from "./helpers/env"; +import { AUTO_DELETE_EVERY_N_HOURS, outputDir, uploadsDir, WEBROOT } from "./helpers/env"; import "./helpers/printVersions"; import { getLandlockRunnerPath, isSandboxAvailable } from "./helpers/sandbox"; import { chooseConverter } from "./pages/chooseConverter"; @@ -20,8 +20,6 @@ import { root } from "./pages/root"; import { upload } from "./pages/upload"; import { user } from "./pages/user"; -export const uploadsDir = "./data/uploads/"; -export const outputDir = "./data/output/"; // Fix for Elysia issue with Bun, (see https://github.com/oven-sh/bun/issues/12161) process.getBuiltinModule = require; diff --git a/src/native/landlock-runner.c b/src/native/landlock-runner.c index b3dff1b9..7b9e47cc 100644 --- a/src/native/landlock-runner.c +++ b/src/native/landlock-runner.c @@ -226,28 +226,45 @@ int main(int argc, char *argv[]) { return 127; } - // Build supported filesystem access rights + // Handled filesystem access rights in ruleset + __u64 handled_fs_access = LANDLOCK_ACCESS_FS_EXECUTE | + LANDLOCK_ACCESS_FS_READ_FILE | + LANDLOCK_ACCESS_FS_READ_DIR | + LANDLOCK_ACCESS_FS_WRITE_FILE | + LANDLOCK_ACCESS_FS_REMOVE_DIR | + LANDLOCK_ACCESS_FS_REMOVE_FILE | + LANDLOCK_ACCESS_FS_MAKE_CHAR | + LANDLOCK_ACCESS_FS_MAKE_DIR | + LANDLOCK_ACCESS_FS_MAKE_REG | + LANDLOCK_ACCESS_FS_MAKE_SOCK | + LANDLOCK_ACCESS_FS_MAKE_FIFO | + LANDLOCK_ACCESS_FS_MAKE_BLOCK | + LANDLOCK_ACCESS_FS_MAKE_SYM; + if (abi >= 2) handled_fs_access |= LANDLOCK_ACCESS_FS_REFER; + if (abi >= 3) handled_fs_access |= LANDLOCK_ACCESS_FS_TRUNCATE; + if (abi >= 5) handled_fs_access |= LANDLOCK_ACCESS_FS_IOCTL_DEV; + + // Build supported filesystem access rights for read-only paths __u64 fs_ro_access = LANDLOCK_ACCESS_FS_EXECUTE | LANDLOCK_ACCESS_FS_READ_FILE | LANDLOCK_ACCESS_FS_READ_DIR; if (abi >= 2) fs_ro_access |= LANDLOCK_ACCESS_FS_REFER; + // Build supported filesystem access rights for read-write paths + // NOTE: Intentionally exclude LANDLOCK_ACCESS_FS_MAKE_SYM, MAKE_CHAR, MAKE_BLOCK, MAKE_FIFO + // to prevent creation of symlinks (CWE-59 sandbox escape) and device nodes. __u64 fs_rw_access = fs_ro_access | LANDLOCK_ACCESS_FS_WRITE_FILE | LANDLOCK_ACCESS_FS_REMOVE_DIR | LANDLOCK_ACCESS_FS_REMOVE_FILE | - LANDLOCK_ACCESS_FS_MAKE_CHAR | LANDLOCK_ACCESS_FS_MAKE_DIR | LANDLOCK_ACCESS_FS_MAKE_REG | - LANDLOCK_ACCESS_FS_MAKE_SOCK | - LANDLOCK_ACCESS_FS_MAKE_FIFO | - LANDLOCK_ACCESS_FS_MAKE_BLOCK | - LANDLOCK_ACCESS_FS_MAKE_SYM; + LANDLOCK_ACCESS_FS_MAKE_SOCK; if (abi >= 3) fs_rw_access |= LANDLOCK_ACCESS_FS_TRUNCATE; if (abi >= 5) fs_rw_access |= LANDLOCK_ACCESS_FS_IOCTL_DEV; struct landlock_ruleset_attr attr = { - .handled_access_fs = fs_rw_access, + .handled_access_fs = handled_fs_access, }; if (no_net && abi >= 4) { diff --git a/src/pages/download.tsx b/src/pages/download.tsx index ffc64e9b..668b7c8d 100644 --- a/src/pages/download.tsx +++ b/src/pages/download.tsx @@ -1,10 +1,10 @@ -import path from "node:path"; import { Elysia } from "elysia"; +import { lstat, realpath } from "node:fs/promises"; +import path from "node:path"; import sanitize from "sanitize-filename"; import * as tar from "tar"; -import { outputDir } from ".."; import db from "../db/db"; -import { WEBROOT } from "../helpers/env"; +import { outputDir, WEBROOT } from "../helpers/env"; import { userService } from "./user"; export const download = new Elysia() @@ -24,13 +24,28 @@ export const download = new Elysia() const jobId = decodeURIComponent(params.jobId); const fileName = sanitize(decodeURIComponent(params.fileName)); - const filePath = `${outputDir}${userId}/${jobId}/${fileName}`; - const file = Bun.file(filePath); - if (!(await file.exists())) { + const jobOutputDir = path.join(outputDir, userId, jobId); + const filePath = path.join(jobOutputDir, fileName); + + try { + const stat = await lstat(filePath); + if (stat.isSymbolicLink()) { + set.status = 403; + return { message: "Access denied." }; + } + + const resolvedBase = await realpath(jobOutputDir); + const resolvedFile = await realpath(filePath); + if (!resolvedFile.startsWith(resolvedBase + path.sep)) { + set.status = 403; + return { message: "Access denied." }; + } + } catch { set.status = 404; return { message: "Converted file not found." }; } + const file = Bun.file(filePath); return file; }, { @@ -50,15 +65,23 @@ export const download = new Elysia() } const jobId = decodeURIComponent(params.jobId); - const outputPath = `${outputDir}${userId}/${jobId}`; + const outputPath = path.join(outputDir, userId, jobId); const outputTar = path.join(outputPath, `converted_files_${jobId}.tar`); await tar.create( { file: outputTar, cwd: outputPath, - filter: (path) => { - return !path.match(".*\\.tar"); + filter: (entryPath, stat) => { + const isSymlink = + ("isSymbolicLink" in stat && + typeof stat.isSymbolicLink === "function" && + stat.isSymbolicLink()) || + ("type" in stat && (stat as { type?: string }).type === "SymbolicLink"); + if (entryPath.endsWith(".tar") || isSymlink) { + return false; + } + return true; }, }, ["."], diff --git a/tests/db/db.test.ts b/tests/db/db.test.ts index 49ec9420..96e103ca 100644 --- a/tests/db/db.test.ts +++ b/tests/db/db.test.ts @@ -1,6 +1,6 @@ -import { test, expect, beforeEach, afterEach, afterAll } from "bun:test"; import { Database } from "bun:sqlite"; -import { unlinkSync, existsSync, mkdirSync } from "node:fs"; +import { afterAll, afterEach, beforeEach, expect, test } from "bun:test"; +import { existsSync, mkdirSync, unlinkSync } from "node:fs"; // set environment variable to ensure the test database is used instead of production data process.env.DB_PATH = "./data/test-isolated.sqlite"; @@ -98,10 +98,7 @@ afterEach(() => { }); afterAll(() => { - // Close the module-level default database before cleanup to prevent file lock errors - if (defaultDb) { - defaultDb.close(); - } + // Do not close defaultDb as subsequent test files in the test suite share the db instance // Cleanup of the isolated test database after the test run if (existsSync("./data/test-isolated.sqlite")) { unlinkSync("./data/test-isolated.sqlite"); diff --git a/tests/helpers/sandbox.test.ts b/tests/helpers/sandbox.test.ts index d451cbba..e00cff70 100644 --- a/tests/helpers/sandbox.test.ts +++ b/tests/helpers/sandbox.test.ts @@ -153,7 +153,19 @@ test("landlock-runner enforces real sandboxing when available", async () => { expect(failure).toBe(true); expect(existsSync(forbiddenFile)).toBe(false); - // 3. Network operations must be blocked (Permission denied) + // 3. Creating symlinks in outputDir must be blocked (Permission denied) + const symlinkTarget = "/etc/passwd"; + const symlinkFile = join(outputDir, "symlink-test"); + const symlinkFailed = await new Promise((resolve) => { + sandboxedExec("/bin/ln", ["-s", symlinkTarget, symlinkFile], (err) => { + resolve(!!err); + }); + }); + + expect(symlinkFailed).toBe(true); + expect(existsSync(symlinkFile)).toBe(false); + + // 4. Network operations must be blocked (Permission denied) const netBlocked = await new Promise((resolve) => { sandboxedExec( "/usr/bin/python3", diff --git a/tests/pages/download.test.ts b/tests/pages/download.test.ts new file mode 100644 index 00000000..51841938 --- /dev/null +++ b/tests/pages/download.test.ts @@ -0,0 +1,90 @@ +import { jwt } from "@elysiajs/jwt"; +import { expect, test } from "bun:test"; +import { Elysia } from "elysia"; +import { mkdirSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import * as tar from "tar"; +import db from "../../src/db/db"; +import { download } from "../../src/pages/download"; + +test("download route blocks symlinks and serves normal files", async () => { + const testUserId = 998; + const testUserEmail = `download-test-${Date.now()}@example.com`; + + db.query("INSERT INTO users (id, email, password) VALUES (?, ?, ?)").run( + testUserId, + testUserEmail, + "hashed_pwd", + ); + + const res = db + .query( + "INSERT INTO jobs (user_id, date_created, status, num_files) VALUES (?, ?, ?, ?) RETURNING id", + ) + .get(testUserId, new Date().toISOString(), "completed", 1) as { id: number }; + const jobId = String(res.id); + + const jobDir = `./data/output/${testUserId}/${jobId}`; + mkdirSync(jobDir, { recursive: true }); + writeFileSync(`${jobDir}/normal.txt`, "safe content"); + symlinkSync("/etc/passwd", `${jobDir}/symlink.txt`); + + try { + const tokenApp = new Elysia() + .use(jwt({ name: "jwt", secret: process.env.JWT_SECRET ?? "secret", exp: "7d" })) + .get("/token", async ({ jwt }) => jwt.sign({ id: String(testUserId) })); + const tokenRes = await tokenApp.handle(new Request("http://localhost/token")); + const token = await tokenRes.text(); + + const app = new Elysia().use(download); + + // 1. Downloading normal file must succeed (200) + const normalRes = await app.handle( + new Request(`http://localhost/download/${testUserId}/${jobId}/normal.txt`, { + headers: { cookie: `auth=${token}` }, + }), + ); + expect(normalRes.status).toBe(200); + expect(await normalRes.text()).toBe("safe content"); + + // 2. Downloading a symlink must be blocked (403) + const symlinkRes = await app.handle( + new Request(`http://localhost/download/${testUserId}/${jobId}/symlink.txt`, { + headers: { cookie: `auth=${token}` }, + }), + ); + expect(symlinkRes.status).toBe(403); + const body = (await symlinkRes.json()) as { message?: string }; + expect(body.message).toBe("Access denied."); + + // 3. Downloading non-existent file must return 404 + const notFoundRes = await app.handle( + new Request(`http://localhost/download/${testUserId}/${jobId}/nonexistent.txt`, { + headers: { cookie: `auth=${token}` }, + }), + ); + expect(notFoundRes.status).toBe(404); + + // 4. Archive tarball must exclude symlinks + const archiveRes = await app.handle( + new Request(`http://localhost/archive/${jobId}`, { + headers: { cookie: `auth=${token}` }, + }), + ); + expect(archiveRes.status).toBe(200); + + const tarEntries: string[] = []; + await tar.t({ + file: `${jobDir}/converted_files_${jobId}.tar`, + onReadEntry: (e) => { + tarEntries.push(e.path); + }, + }); + + expect(tarEntries).toContain("./normal.txt"); + expect(tarEntries).not.toContain("./symlink.txt"); + } finally { + rmSync(`./data/output/${testUserId}`, { recursive: true, force: true }); + db.query("DELETE FROM jobs WHERE id = ?").run(jobId); + db.query("DELETE FROM users WHERE id = ?").run(testUserId); + } +});