Merge pull request #4862 from Dokploy/fix/cmdi-db-backup-restore

fix(security): OS command injection in database backup/restore commands

apps/dokploy/__test__/backups/db-backup-restore-injection.test.ts

@@ -0,0 +1,106 @@
+import { execSync } from "node:child_process";
+import { chmodSync, existsSync, rmSync, writeFileSync } from "node:fs";
+import {
+	getLibsqlBackupCommand,
+	getMariadbBackupCommand,
+	getMongoBackupCommand,
+	getMysqlBackupCommand,
+	getPostgresBackupCommand,
+} from "@dokploy/server/utils/backups/utils";
+import {
+	getMariadbRestoreCommand,
+	getMongoRestoreCommand,
+	getMysqlRestoreCommand,
+	getPostgresRestoreCommand,
+} from "@dokploy/server/utils/restore/utils";
+import { afterAll, beforeAll, describe, expect, it } from "vitest";
+
+// A stub replacing the real `docker` binary. It ignores exec/-i/$CONTAINER_ID,
+// exports the -e VAR=val pairs, and runs the inner `sh -c <script>` — so the
+// test exercises BOTH shell layers (outer /bin/sh building the docker command,
+// and the inner shell) the way production does, without needing a container.
+const stub = `/tmp/docker_stub_${process.pid}`;
+const MARK = `/tmp/dokploy_dbbk_pwned_${process.pid}`;
+
+beforeAll(() => {
+	writeFileSync(
+		stub,
+		`#!/bin/bash
+shift # exec
+envs=()
+while [ "$1" = "-e" ]; do envs+=("$2"); shift 2; done
+shift 2 # -i CONTAINER
+shell="$1"; shift # bash|sh
+shift # -c
+env "\${envs[@]}" "$shell" -c "$1" </dev/null 2>/dev/null || true
+`,
+	);
+	chmodSync(stub, 0o755);
+});
+
+afterAll(() => {
+	if (existsSync(stub)) rmSync(stub);
+	if (existsSync(MARK)) rmSync(MARK);
+});
+
+// Run a builder-produced command with `docker` pointed at the stub; return true
+// if no injected command fired.
+const runsSafely = (command: string) => {
+	if (existsSync(MARK)) rmSync(MARK);
+	const withStub = command.replace(/^docker /, `${stub} `);
+	try {
+		execSync(withStub, {
+			shell: "/bin/bash",
+			stdio: "ignore",
+			env: { ...process.env, CONTAINER_ID: "test" },
+		});
+	} catch {}
+	const fired = existsSync(MARK);
+	if (existsSync(MARK)) rmSync(MARK);
+	return !fired;
+};
+
+// Payloads that try to break out of every quoting style used in the builders.
+const p = (mark: string) => [
+	`$(touch ${mark})`,
+	"`touch " + mark + "`",
+	`x'; touch ${mark}; '`,
+	`x"; touch ${mark}; echo "`,
+	`x; touch ${mark}`,
+];
+
+describe("database backup/restore command injection", () => {
+	const cases: Array<[string, (v: string) => string]> = [
+		["postgres backup (database)", (v) => getPostgresBackupCommand(v, "u")],
+		["postgres backup (user)", (v) => getPostgresBackupCommand("db", v)],
+		["mariadb backup (password)", (v) => getMariadbBackupCommand("db", "u", v)],
+		["mysql backup (database)", (v) => getMysqlBackupCommand(v, "pw")],
+		["mongo backup (user)", (v) => getMongoBackupCommand("db", v, "pw")],
+		["libsql backup (database)", (v) => getLibsqlBackupCommand(v)],
+		["postgres restore (database)", (v) => getPostgresRestoreCommand(v, "u")],
+		[
+			"mariadb restore (password)",
+			(v) => getMariadbRestoreCommand("db", "u", v),
+		],
+		["mysql restore (database)", (v) => getMysqlRestoreCommand(v, "pw")],
+		["mongo restore (user)", (v) => getMongoRestoreCommand("db", v, "pw")],
+	];
+
+	for (const [label, build] of cases) {
+		it(`${label} is not injectable`, () => {
+			for (const payload of p(MARK)) {
+				expect(runsSafely(build(payload))).toBe(true);
+			}
+		});
+	}
+
+	it("preserves a legitimate database name (passed through as env var)", () => {
+		const cmd = getPostgresBackupCommand("my-db_prod", "app_user");
+		// Values live in -e assignments, never inline in the pg_dump text.
+		expect(cmd).toContain("-e DB_NAME=my-db_prod");
+		expect(cmd).toContain("-e DB_USER=app_user");
+		expect(cmd).toContain(
+			'pg_dump -Fc --no-acl --no-owner -h localhost -U "$DB_USER"',
+		);
+	});
+});

packages/server/src/utils/backups/utils.ts

@@ -2,6 +2,7 @@ import { logger } from "@dokploy/server/lib/logger";
 import type { BackupSchedule } from "@dokploy/server/services/backup";
 import type { Destination } from "@dokploy/server/services/destination";
 import { scheduledJobs, scheduleJob } from "node-schedule";
+import { quote } from "shell-quote";
 import { keepLatestNBackups } from ".";
 import { runComposeBackup } from "./compose";
 import { runLibsqlBackup } from "./libsql";
@@ -90,11 +91,16 @@ export const getS3Credentials = (destination: Destination) => {
 	return rcloneFlags;
 };
 
+// User-controlled values (database name, user, password) are passed to the
+// container as environment variables via `docker exec -e VAR=<escaped>` and
+// referenced as "$VAR" inside the inner shell, so they never appear in the
+// inner command text. The -e value is escaped for the outer shell with
+// shell-quote; the inner script is single-quoted and reads the env vars.
 export const getPostgresBackupCommand = (
 	database: string,
 	databaseUser: string,
 ) => {
-	return `docker exec -i $CONTAINER_ID bash -c "set -o pipefail; pg_dump -Fc --no-acl --no-owner -h localhost -U ${databaseUser} --no-password '${database}' | gzip"`;
+	return `docker exec -e DB_NAME=${quote([database])} -e DB_USER=${quote([databaseUser])} -i $CONTAINER_ID bash -c 'set -o pipefail; pg_dump -Fc --no-acl --no-owner -h localhost -U "$DB_USER" --no-password "$DB_NAME" | gzip'`;
 };
 
 export const getMariadbBackupCommand = (
@@ -102,14 +108,14 @@ export const getMariadbBackupCommand = (
 	databaseUser: string,
 	databasePassword: string,
 ) => {
-	return `docker exec -i $CONTAINER_ID bash -c "set -o pipefail; mariadb-dump --user='${databaseUser}' --password='${databasePassword}' --single-transaction --quick --databases ${database} | gzip"`;
+	return `docker exec -e DB_NAME=${quote([database])} -e DB_USER=${quote([databaseUser])} -e DB_PASS=${quote([databasePassword])} -i $CONTAINER_ID bash -c 'set -o pipefail; mariadb-dump --user="$DB_USER" --password="$DB_PASS" --single-transaction --quick --databases "$DB_NAME" | gzip'`;
 };
 
 export const getMysqlBackupCommand = (
 	database: string,
 	databasePassword: string,
 ) => {
-	return `docker exec -i $CONTAINER_ID bash -c "set -o pipefail; mysqldump --default-character-set=utf8mb4 -u 'root' --password='${databasePassword}' --single-transaction --no-tablespaces --quick '${database}' | gzip"`;
+	return `docker exec -e DB_NAME=${quote([database])} -e DB_PASS=${quote([databasePassword])} -i $CONTAINER_ID bash -c 'set -o pipefail; mysqldump --default-character-set=utf8mb4 -u root --password="$DB_PASS" --single-transaction --no-tablespaces --quick "$DB_NAME" | gzip'`;
 };
 
 export const getMongoBackupCommand = (
@@ -117,11 +123,11 @@ export const getMongoBackupCommand = (
 	databaseUser: string,
 	databasePassword: string,
 ) => {
-	return `docker exec -i $CONTAINER_ID bash -c "set -o pipefail; mongodump -d '${database}' -u '${databaseUser}' -p '${databasePassword}' --archive --authenticationDatabase admin --gzip"`;
+	return `docker exec -e DB_NAME=${quote([database])} -e DB_USER=${quote([databaseUser])} -e DB_PASS=${quote([databasePassword])} -i $CONTAINER_ID bash -c 'set -o pipefail; mongodump -d "$DB_NAME" -u "$DB_USER" -p "$DB_PASS" --archive --authenticationDatabase admin --gzip'`;
 };
 
 export const getLibsqlBackupCommand = (database: string) => {
-	return `docker exec -i $CONTAINER_ID sh -c "tar cf - -C /var/lib/sqld ${database} | gzip"`;
+	return `docker exec -e DB_NAME=${quote([database])} -i $CONTAINER_ID sh -c 'tar cf - -C /var/lib/sqld "$DB_NAME" | gzip'`;
 };
 
 export const getServiceContainerCommand = (appName: string) => {

packages/server/src/utils/restore/utils.ts

@@ -1,13 +1,17 @@
+import { quote } from "shell-quote";
 import {
 	getComposeContainerCommand,
 	getServiceContainerCommand,
 } from "../backups/utils";
 
+// User-controlled values are passed to the container via `docker exec -e` and
+// read as "$VAR" inside a single-quoted inner script, so they never enter the
+// inner command text. See the matching note in backups/utils.ts.
 export const getPostgresRestoreCommand = (
 	database: string,
 	databaseUser: string,
 ) => {
-	return `docker exec -i $CONTAINER_ID sh -c "pg_restore -U '${databaseUser}' -d ${database} -O --clean --if-exists"`;
+	return `docker exec -e DB_NAME=${quote([database])} -e DB_USER=${quote([databaseUser])} -i $CONTAINER_ID sh -c 'pg_restore -U "$DB_USER" -d "$DB_NAME" -O --clean --if-exists'`;
 };
 
 export const getMariadbRestoreCommand = (
@@ -15,14 +19,14 @@ export const getMariadbRestoreCommand = (
 	databaseUser: string,
 	databasePassword: string,
 ) => {
-	return `docker exec -i $CONTAINER_ID sh -c "mariadb -u '${databaseUser}' -p'${databasePassword}' ${database}"`;
+	return `docker exec -e DB_NAME=${quote([database])} -e DB_USER=${quote([databaseUser])} -e DB_PASS=${quote([databasePassword])} -i $CONTAINER_ID sh -c 'mariadb -u "$DB_USER" -p"$DB_PASS" "$DB_NAME"'`;
 };
 
 export const getMysqlRestoreCommand = (
 	database: string,
 	databasePassword: string,
 ) => {
-	return `docker exec -i $CONTAINER_ID sh -c "mysql -u root -p'${databasePassword}' ${database}"`;
+	return `docker exec -e DB_NAME=${quote([database])} -e DB_PASS=${quote([databasePassword])} -i $CONTAINER_ID sh -c 'mysql -u root -p"$DB_PASS" "$DB_NAME"'`;
 };
 
 export const getMongoRestoreCommand = (
@@ -30,7 +34,7 @@ export const getMongoRestoreCommand = (
 	databaseUser: string,
 	databasePassword: string,
 ) => {
-	return `docker exec -i $CONTAINER_ID sh -c "mongorestore --username '${databaseUser}' --password '${databasePassword}' --authenticationDatabase admin --db ${database} --archive --drop"`;
+	return `docker exec -e DB_NAME=${quote([database])} -e DB_USER=${quote([databaseUser])} -e DB_PASS=${quote([databasePassword])} -i $CONTAINER_ID sh -c 'mongorestore --username "$DB_USER" --password "$DB_PASS" --authenticationDatabase admin --db "$DB_NAME" --archive --drop'`;
 };
 
 export const getComposeSearchCommand = (