From b4fbd46cc1b9e319f88748d7ac46d0677be43e75 Mon Sep 17 00:00:00 2001 From: Marek Miklewicz Date: Sat, 4 Jul 2026 22:53:21 +0200 Subject: [PATCH] Harden restore-archive extraction against path traversal extract_payload_from_archive() relied entirely on tar's own default path-traversal protection when extracting a DirectAdmin restore archive to locate the plugin's backup/alt_mysql payload. Explicitly inspect the archive listing first and refuse to extract at all if any member has an absolute path or a ".." segment, rather than trusting that the wildcard extraction pattern happens to filter out an unsafe member. --- ...alt_mysql_user_restore_post_pre_cleanup.sh | 15 +++- tests/da_restore_archive_traversal_test.sh | 70 +++++++++++++++++++ 2 files changed, 83 insertions(+), 2 deletions(-) create mode 100755 tests/da_restore_archive_traversal_test.sh diff --git a/scripts/da-integration/alt_mysql_user_restore_post_pre_cleanup.sh b/scripts/da-integration/alt_mysql_user_restore_post_pre_cleanup.sh index 1882075..6636209 100755 --- a/scripts/da-integration/alt_mysql_user_restore_post_pre_cleanup.sh +++ b/scripts/da-integration/alt_mysql_user_restore_post_pre_cleanup.sh @@ -85,13 +85,24 @@ find_payload_in_dir() { extract_payload_from_archive() { local archive="$1" - local tmp_dir payload_member + local tmp_dir payload_member listing [ -r "$archive" ] || return 1 command -v tar >/dev/null 2>&1 || return 1 - payload_member="$(tar -tf "$archive" 2>/dev/null | grep -E '(^|/)backup/alt_mysql/manifest\.json$' | head -n 1 || true)" + listing="$(tar -tf "$archive" 2>/dev/null || true)" + [ -n "$listing" ] || return 1 + + payload_member="$(printf '%s\n' "$listing" | grep -E '(^|/)backup/alt_mysql/manifest\.json$' | head -n 1 || true)" [ -n "$payload_member" ] || return 1 + # Security: never trust tar's own default path-traversal protection alone - + # refuse to extract archives containing any absolute-path or ".." member, + # even if the offending member wouldn't otherwise match our own wildcard. + if printf '%s\n' "$listing" | grep -Eq '(^|/)\.\.(/|$)|^/'; then + log "refusing to extract archive with unsafe member paths: $archive" + return 1 + fi + tmp_dir="$(mktemp -d)" tar -xf "$archive" -C "$tmp_dir" --wildcards '*/backup/alt_mysql/*' 'backup/alt_mysql/*' 2>/dev/null \ || tar -xf "$archive" -C "$tmp_dir" 2>/dev/null diff --git a/tests/da_restore_archive_traversal_test.sh b/tests/da_restore_archive_traversal_test.sh new file mode 100755 index 0000000..2a98f87 --- /dev/null +++ b/tests/da_restore_archive_traversal_test.sh @@ -0,0 +1,70 @@ +#!/bin/bash +set -euo pipefail + +PLUGIN_DIR="$(cd "$(dirname "$0")/.." && pwd)" +REAL_SCRIPT="$PLUGIN_DIR/scripts/da-integration/alt_mysql_user_restore_post_pre_cleanup.sh" +TMP_DIR="$(mktemp -d)" +trap 'rm -rf "$TMP_DIR" "/tmp/alt-mysql-traversal-proof"' EXIT + +fail() { + echo "FAIL: $*" >&2 + exit 1 +} + +# Extract the exact, real find_payload_in_dir()/extract_payload_from_archive() +# function bodies from the real script (rather than reimplementing them), so +# this test exercises the actual production logic without triggering the +# script's own main() (which needs a full DA restore environment). +FUNCS_FILE="$TMP_DIR/funcs.sh" +{ + echo '#!/bin/bash' + echo 'log() { :; }' + awk '/^find_payload_in_dir\(\)/,/^\}/' "$REAL_SCRIPT" + awk '/^extract_payload_from_archive\(\)/,/^\}/' "$REAL_SCRIPT" +} > "$FUNCS_FILE" + +grep -q '^find_payload_in_dir()' "$FUNCS_FILE" || fail "could not extract find_payload_in_dir() from $REAL_SCRIPT" +grep -q '^extract_payload_from_archive()' "$FUNCS_FILE" || fail "could not extract extract_payload_from_archive() from $REAL_SCRIPT" + +# shellcheck source=/dev/null +source "$FUNCS_FILE" + +# --- A legitimate archive must still extract normally --- +GOOD_STAGE="$TMP_DIR/good_stage" +mkdir -p "$GOOD_STAGE/backup/alt_mysql/databases" +printf 'x' > "$GOOD_STAGE/backup/alt_mysql/manifest.json" +printf 'x' > "$GOOD_STAGE/backup/alt_mysql/databases/demo_db.sql.gz" +GOOD_ARCHIVE="$TMP_DIR/good.tar" +( cd "$GOOD_STAGE" && tar -cf "$GOOD_ARCHIVE" backup/alt_mysql/manifest.json backup/alt_mysql/databases/demo_db.sql.gz ) + +GOOD_PAYLOAD="$(extract_payload_from_archive "$GOOD_ARCHIVE")" \ + || fail "a legitimate archive with a valid manifest must be extracted" +[ -r "$GOOD_PAYLOAD/manifest.json" ] || fail "extracted payload must contain manifest.json" + +# --- An archive containing a path-traversal member must be refused entirely --- +EVIL_STAGE="$TMP_DIR/evil_stage" +mkdir -p "$EVIL_STAGE/backup/alt_mysql" "$EVIL_STAGE/evil_payload" +printf 'x' > "$EVIL_STAGE/backup/alt_mysql/manifest.json" +printf 'malicious content' > "$EVIL_STAGE/evil_payload/evil.sh" +EVIL_ARCHIVE="$TMP_DIR/evil.tar" + +( + cd "$EVIL_STAGE" && { + tar -cf "$EVIL_ARCHIVE" -s ',^evil_payload/,../../../../../../tmp/alt-mysql-traversal-proof/,' \ + backup/alt_mysql/manifest.json evil_payload/evil.sh 2>/dev/null \ + || tar -cf "$EVIL_ARCHIVE" --transform 's,^evil_payload/,../../../../../../tmp/alt-mysql-traversal-proof/,' \ + backup/alt_mysql/manifest.json evil_payload/evil.sh + } +) + +tar -tf "$EVIL_ARCHIVE" | grep -Fq '../../../../../../tmp/alt-mysql-traversal-proof/evil.sh' \ + || fail "test setup broken: the crafted archive does not actually contain a traversal member" + +EVIL_OUTPUT="" +if EVIL_OUTPUT="$(extract_payload_from_archive "$EVIL_ARCHIVE" 2>&1)"; then + fail "an archive containing a path-traversal member must be refused, got payload: $EVIL_OUTPUT" +fi +[ ! -e "/tmp/alt-mysql-traversal-proof/evil.sh" ] \ + || fail "SECURITY: path-traversal member was actually extracted outside the temp dir" + +echo "da_restore_archive_traversal_test: OK"