Merge pull request #3733 from Dokploy/fix/Remote-Code-Execution-through-Path-Traversal

feat(tests): add unit tests for readValidDirectory function to valida…

Author: Siumauricio

apps/dokploy/__test__/drop/drop.test.ts

@@ -6,14 +6,18 @@ import { paths } from "@dokploy/server/constants";
 import AdmZip from "adm-zip";
 import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
 
+const OUTPUT_BASE = "./__test__/drop/zips/output";
 const { APPLICATIONS_PATH } = paths();
 vi.mock("@dokploy/server/constants", async (importOriginal) => {
 	const actual = await importOriginal();
 	return {
 		// @ts-ignore
 		...actual,
 		paths: () => ({
-			APPLICATIONS_PATH: "./__test__/drop/zips/output",
+			// @ts-ignore
+			...actual.paths(),
+			BASE_PATH: OUTPUT_BASE,
+			APPLICATIONS_PATH: OUTPUT_BASE,
 		}),
 	};
 });
@@ -150,6 +154,176 @@ const baseApp: ApplicationNested = {
 	ulimitsSwarm: null,
 };
 
+/**
+ * GHSA-66v7-g3fh-47h3: Remote Code Execution through Path Traversal.
+ * Validates the exact PoC: ZIP with path traversal entry ../../../../../etc/cron.d/malicious-cron
+ * plus cover files (package.json, index.js). unzipDrop must reject and never write outside output.
+ */
+describe("GHSA-66v7-g3fh-47h3 path traversal RCE", () => {
+	beforeAll(async () => {
+		await fs.rm(APPLICATIONS_PATH, { recursive: true, force: true });
+	});
+	afterAll(async () => {
+		await fs.rm(APPLICATIONS_PATH, { recursive: true, force: true });
+	});
+
+	it("rejects PoC ZIP: traversal ../../../../../etc/cron.d/malicious-cron + package.json + index.js", async () => {
+		baseApp.appName = "ghsa-rce";
+		// PoC payload: same entry name as advisory (Python zipfile keeps it; AdmZip normalizes on add → use placeholder + replace)
+		const traversalEntry = "../../../../../etc/cron.d/malicious-cron";
+		const cronPayload = "* * * * * root id\n";
+		const placeholder = "x".repeat(traversalEntry.length);
+		const zip = new AdmZip();
+		zip.addFile(
+			"package.json",
+			Buffer.from('{"name": "app", "version": "1.0.0"}'),
+		);
+		zip.addFile("index.js", Buffer.from('console.log("Application");'));
+		zip.addFile(placeholder, Buffer.from(cronPayload));
+		let buf = Buffer.from(zip.toBuffer());
+		buf = Buffer.from(
+			buf.toString("binary").split(placeholder).join(traversalEntry),
+			"binary",
+		);
+		const file = new File([buf as unknown as ArrayBuffer], "exploit.zip");
+		await expect(unzipDrop(file, baseApp)).rejects.toThrow(
+			/Path traversal detected.*resolved path escapes output directory/,
+		);
+	});
+});
+
+describe("security: existing symlink escape", () => {
+	beforeAll(async () => {
+		await fs.rm(APPLICATIONS_PATH, { recursive: true, force: true });
+	});
+
+	afterAll(async () => {
+		await fs.rm(APPLICATIONS_PATH, { recursive: true, force: true });
+	});
+
+	it("should NOT write outside base when directory is a symlink", async () => {
+		const appName = "symlink-existing";
+		const output = path.join(APPLICATIONS_PATH, appName, "code");
+		await fs.mkdir(output, { recursive: true });
+
+		// outside target (attacker wants to write here)
+		const outside = path.join(APPLICATIONS_PATH, "..", "outside");
+		await fs.mkdir(outside, { recursive: true });
+
+		// attacker-controlled symlink inside project
+		await fs.symlink(outside, path.join(output, "logs"));
+
+		// zip looks totally harmless
+		const zip = new AdmZip();
+		zip.addFile("logs/pwned.txt", Buffer.from("owned"));
+
+		const file = new File([zip.toBuffer() as any], "exploit.zip");
+
+		await unzipDrop(file, { ...baseApp, appName });
+
+		// if vulnerable -> file exists outside sandbox
+		const escaped = await fs
+			.readFile(path.join(outside, "pwned.txt"), "utf8")
+			.then(() => true)
+			.catch(() => false);
+
+		expect(escaped).toBe(false);
+	});
+});
+
+describe("security: zip symlink entry blocked", () => {
+	beforeAll(async () => {
+		await fs.rm(APPLICATIONS_PATH, { recursive: true, force: true });
+	});
+
+	afterAll(async () => {
+		await fs.rm(APPLICATIONS_PATH, { recursive: true, force: true });
+	});
+
+	it("rejects zip containing real symlink entry", async () => {
+		const appName = "zip-symlink";
+
+		const zipBuffer = await fs.readFile(
+			path.join(__dirname, "./zips/payload/symlink-entry.zip"),
+		);
+
+		const file = new File([zipBuffer as any], "exploit.zip");
+
+		await expect(unzipDrop(file, { ...baseApp, appName })).rejects.toThrow(
+			/Dangerous node entries are not allowed/,
+		);
+	});
+});
+
+describe("unzipDrop path under output (no traversal)", () => {
+	beforeAll(async () => {
+		await fs.rm(APPLICATIONS_PATH, { recursive: true, force: true });
+	});
+	afterAll(async () => {
+		await fs.rm(APPLICATIONS_PATH, { recursive: true, force: true });
+	});
+
+	it("allows entry etc/cron.d/malicious-cron when under output (no path traversal)", async () => {
+		baseApp.appName = "cron-under-output";
+		const zip = new AdmZip();
+		zip.addFile(
+			"etc/cron.d/malicious-cron",
+			Buffer.from("* * * * * root id\n"),
+		);
+		zip.addFile("package.json", Buffer.from('{"name":"app"}'));
+		const file = new File(
+			[zip.toBuffer() as unknown as ArrayBuffer],
+			"app.zip",
+		);
+		const outputPath = path.join(APPLICATIONS_PATH, baseApp.appName, "code");
+		await unzipDrop(file, baseApp);
+		const content = await fs.readFile(
+			path.join(outputPath, "etc/cron.d/malicious-cron"),
+			"utf8",
+		);
+		expect(content).toBe("* * * * * root id\n");
+	});
+});
+
+describe("security: traversal inside BASE_PATH (sandbox escape)", () => {
+	beforeAll(async () => {
+		await fs.rm(APPLICATIONS_PATH, { recursive: true, force: true });
+	});
+
+	afterAll(async () => {
+		await fs.rm(APPLICATIONS_PATH, { recursive: true, force: true });
+	});
+
+	it("should NOT allow writing outside application directory but inside BASE_PATH", async () => {
+		const appName = "sandbox-escape";
+
+		const base = APPLICATIONS_PATH.replace("/applications", "");
+		const output = path.join(APPLICATIONS_PATH, appName, "code");
+
+		await fs.mkdir(output, { recursive: true });
+
+		// attacker writes into traefik config inside base
+		const zip = new AdmZip();
+		zip.addFile(
+			"../../../traefik/dynamic/evil.yml",
+			Buffer.from("pwned: true"),
+		);
+
+		const file = new File([zip.toBuffer() as any], "exploit.zip");
+
+		await unzipDrop(file, { ...baseApp, appName });
+
+		const escapedPath = path.join(base, "traefik/dynamic/evil.yml");
+
+		const exists = await fs
+			.readFile(escapedPath)
+			.then(() => true)
+			.catch(() => false);
+
+		expect(exists).toBe(false);
+	});
+});
+
 describe("unzipDrop using real zip files", () => {
 	// const { APPLICATIONS_PATH } = paths();
 	beforeAll(async () => {
@@ -166,14 +340,12 @@ describe("unzipDrop using real zip files", () => {
 		try {
 			const outputPath = path.join(APPLICATIONS_PATH, baseApp.appName, "code");
 			const zip = new AdmZip("./__test__/drop/zips/single-file.zip");
-			console.log(`Output Path: ${outputPath}`);
 			const zipBuffer = zip.toBuffer() as Buffer<ArrayBuffer>;
 			const file = new File([zipBuffer], "single.zip");
 			await unzipDrop(file, baseApp);
 			const files = await fs.readdir(outputPath, { withFileTypes: true });
 			expect(files.some((f) => f.name === "test.txt")).toBe(true);
 		} catch (err) {
-			console.log(err);
 		} finally {
 		}
 	});

apps/dokploy/__test__/drop/zips/payload/link

@@ -0,0 +1 @@
+/etc/passwd

apps/dokploy/__test__/drop/zips/payload/symlink-entry.zip

@@ -0,0 +1,81 @@
+import path from "node:path";
+import { describe, expect, it, vi } from "vitest";
+
+const BASE = "/base";
+
+vi.mock("@dokploy/server/constants", async (importOriginal) => {
+	const actual =
+		await importOriginal<typeof import("@dokploy/server/constants")>();
+	return {
+		...actual,
+		paths: () => ({
+			...actual.paths(),
+			BASE_PATH: BASE,
+			LOGS_PATH: `${BASE}/logs`,
+			APPLICATIONS_PATH: `${BASE}/applications`,
+		}),
+	};
+});
+
+// Import after mock so paths() uses our BASE
+const { readValidDirectory } = await import("@dokploy/server");
+
+describe("readValidDirectory (path traversal)", () => {
+	it("returns true when directory is exactly BASE_PATH", () => {
+		expect(readValidDirectory(BASE)).toBe(true);
+		expect(readValidDirectory(path.resolve(BASE))).toBe(true);
+	});
+
+	it("returns true when directory is under BASE_PATH", () => {
+		expect(readValidDirectory(`${BASE}/logs`)).toBe(true);
+		expect(readValidDirectory(`${BASE}/logs/app/foo.log`)).toBe(true);
+		expect(readValidDirectory(`${BASE}/applications/myapp/code`)).toBe(true);
+	});
+
+	it("returns false for path traversal escaping base (absolute)", () => {
+		expect(readValidDirectory("/etc/passwd")).toBe(false);
+		expect(readValidDirectory("/etc/cron.d/malicious")).toBe(false);
+		expect(readValidDirectory("/tmp/outside")).toBe(false);
+	});
+
+	it("returns false when resolved path escapes base via ..", () => {
+		// Resolved: /etc/passwd (outside /base)
+		expect(readValidDirectory(`${BASE}/../etc/passwd`)).toBe(false);
+		expect(readValidDirectory(`${BASE}/logs/../../etc/passwd`)).toBe(false);
+		expect(readValidDirectory(`${BASE}/..`)).toBe(false);
+	});
+
+	it("returns true when .. stays within base", () => {
+		// e.g. /base/logs/../applications -> /base/applications (still under /base)
+		expect(readValidDirectory(`${BASE}/logs/../applications`)).toBe(true);
+		expect(readValidDirectory(`${BASE}/foo/../bar`)).toBe(true);
+	});
+
+	it("accepts serverId for remote base path", () => {
+		// With our mock, serverId doesn't change BASE_PATH; just ensure it doesn't throw
+		expect(readValidDirectory(BASE, "server-1")).toBe(true);
+		expect(readValidDirectory("/etc/passwd", "server-1")).toBe(false);
+	});
+
+	it("returns false for null/undefined-like paths that resolve outside", () => {
+		// Paths that might resolve to cwd or root
+		expect(readValidDirectory(".")).toBe(false);
+		expect(readValidDirectory("..")).toBe(false);
+	});
+
+	it("returns true for BASE_PATH with trailing slash or double slashes under base", () => {
+		expect(readValidDirectory(`${BASE}/`)).toBe(true);
+		expect(readValidDirectory(`${BASE}//logs`)).toBe(true);
+		expect(readValidDirectory(`${BASE}/applications///myapp/code`)).toBe(true);
+	});
+
+	it("returns false when path looks like base but is a sibling or prefix", () => {
+		expect(readValidDirectory("/base-evil")).toBe(false);
+		expect(readValidDirectory("/bas")).toBe(false);
+		expect(readValidDirectory(`${BASE}/../base-evil`)).toBe(false);
+	});
+
+	it("returns false for empty string (resolves to cwd)", () => {
+		expect(readValidDirectory("")).toBe(false);
+	});
+});

apps/dokploy/__test__/wss/readValidDirectory.test.ts

@@ -1,9 +1,9 @@
 import { spawn } from "node:child_process";
 import type http from "node:http";
 import { findServerById, IS_CLOUD, validateRequest } from "@dokploy/server";
+import { readValidDirectory } from "@dokploy/server/wss/utils";
 import { Client } from "ssh2";
 import { WebSocketServer } from "ws";
-import { readValidDirectory } from "./utils";
 
 export const setupDeploymentLogsWebSocketServer = (
 	server: http.Server<typeof http.IncomingMessage, typeof http.ServerResponse>,

apps/dokploy/server/wss/listen-deployment.ts

@@ -32,20 +32,6 @@ export const isValidShell = (shell: string): boolean => {
 	return allowedShells.includes(shell);
 };
 
-export const readValidDirectory = (
-	directory: string,
-	serverId?: string | null,
-) => {
-	const { BASE_PATH } = paths(!!serverId);
-
-	const resolvedBase = path.resolve(BASE_PATH);
-	const resolvedDir = path.resolve(directory);
-
-	return (
-		resolvedDir === resolvedBase ||
-		resolvedDir.startsWith(resolvedBase + path.sep)
-	);
-};
 export const getShell = () => {
 	if (IS_CLOUD) {
 		return "NO_AVAILABLE";

apps/dokploy/server/wss/utils.ts

@@ -3,6 +3,7 @@ import path, { join } from "node:path";
 import { paths } from "@dokploy/server/constants";
 import type { Application } from "@dokploy/server/services/application";
 import { findServerById } from "@dokploy/server/services/server";
+import { readValidDirectory } from "@dokploy/server/wss/utils";
 import AdmZip from "adm-zip";
 import { Client, type SFTPWrapper } from "ssh2";
 import {
@@ -62,6 +63,17 @@ export const unzipDrop = async (zipFile: File, application: Application) => {
 			if (!filePath) continue;
 
 			const fullPath = path.join(outputPath, filePath).replace(/\\/g, "/");
+			if (!readValidDirectory(fullPath, application.serverId)) {
+				throw new Error(
+					`Path traversal detected: resolved path escapes output directory: ${filePath}`,
+				);
+			}
+
+			if (isDangerousNode(entry)) {
+				throw new Error(
+					`Dangerous node entries are not allowed: ${entry.entryName}`,
+				);
+			}
 
 			if (application.serverId) {
 				if (!entry.isDirectory) {
@@ -132,3 +144,14 @@ const uploadFileToServer = (
 		});
 	});
 };
+
+function isDangerousNode(entry: AdmZip.IZipEntry) {
+	const type = (entry.header.attr >> 16) & 0o170000;
+
+	return (
+		type === 0o120000 || // symlink
+		type === 0o060000 || // block device
+		type === 0o020000 || // char device
+		type === 0o010000 // fifo/pipe
+	);
+}

packages/server/src/utils/builders/drop.ts

@@ -1,4 +1,6 @@
 import os from "node:os";
+import path from "node:path";
+import { paths } from "@dokploy/server/constants";
 import { publicIpv4, publicIpv6 } from "public-ip";
 
 export const getShell = () => {
@@ -33,3 +35,18 @@ export const getPublicIpWithFallback = async () => {
 	}
 	return ip;
 };
+
+export const readValidDirectory = (
+	directory: string,
+	serverId?: string | null,
+) => {
+	const { BASE_PATH } = paths(!!serverId);
+
+	const resolvedBase = path.resolve(BASE_PATH);
+	const resolvedDir = path.resolve(directory);
+
+	return (
+		resolvedDir === resolvedBase ||
+		resolvedDir.startsWith(resolvedBase + path.sep)
+	);
+};