From 72b8fa9d088c4be8a0fcaa518d7b73de13139369 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Tue, 20 Oct 2026 10:47:04 -0700 Subject: [PATCH] extract: post-extract realpath check of every symlink; classifier refuses link chains (N2-1 review) The lexical relative-target check missed chains: `a -> b/../..` with `b -> ../..`, or `l1 -> l2/../../..`, each resolve inside the archive lexically but above the extract through the other link. - scripts/lib/strip-escaping-links.py (new, the authority): after tar, lstat-walk the WHOLE extract (never following links), realpath(strict) every symlink, and remove + record (stripped_escaping_symlink) any that resolves outside the extract or to it, dangles or loops. Repeats to a fixed point; a walk error fails the extract. Own-account absolute /home[N]// targets are kept (they cannot resolve in the sandbox and resolve only inside the customer's site container on WHP). - scan-symlinks.php: two passes; a relative link whose resolution passes through another symlink entry of the archive is DANGEROUS. - fixtures/tests: cpmove-chain (both shapes) and a hand-built tree for the post-extract pass (chains, /etc, dangling .bak sidecar, escaping .bak, loop, link to the top). 3 chain checks fail on a226b49. Co-Authored-By: Claude Opus 5.5 --- scripts/extract.sh | 20 ++++++ scripts/lib/scan-symlinks.php | 28 ++++++-- scripts/lib/strip-escaping-links.py | 102 ++++++++++++++++++++++++++++ tests/build-fixtures.sh | 17 +++++ tests/run-tests.sh | 36 ++++++++++ 5 files changed, 196 insertions(+), 7 deletions(-) create mode 100644 scripts/lib/strip-escaping-links.py diff --git a/scripts/extract.sh b/scripts/extract.sh index 0340dba..9783946 100755 --- a/scripts/extract.sh +++ b/scripts/extract.sh @@ -175,6 +175,23 @@ tar --no-same-owner --no-same-permissions --no-overwrite-dir \ --exclude-from="$EXCLUDES_FILE" \ $TAR_FLAGS "$TARBALL" || TAR_RC=$? +# N2-1 authority for the WHOLE tree: the classifier above resolves relative +# targets lexically, so a chain through another link (a -> b/.., b -> x/y/z) +# can pass it. strip-escaping-links.py asks realpath() where every extracted +# symlink really lands and removes (records) any that leave $DEST, point at +# $DEST itself, dangle or loop — own-account absolute /home// targets +# excepted. A walk error fails the extract closed. +LINK_USERS=("$USERNAME") +for top in "$DEST"/*/; do + [[ -d "$top" && ! -L "${top%/}" ]] || continue + t="$(basename "$top")"; t="${t#cpmove-}"; LINK_USERS+=("$t" "${t##*_}") +done +STRIP_RC=0 +ESCAPING=$(python3 -I "$SCRIPT_DIR/lib/strip-escaping-links.py" "$DEST" "$ACTIONS_OUT" "${LINK_USERS[@]}") || STRIP_RC=$? +if [[ $STRIP_RC -eq 0 && "${ESCAPING:-0}" -gt 0 ]]; then + log "stripped $ESCAPING symlink(s) that resolve outside the extract, dangle or loop (post-extract check)" +fi + VERIFY_RC=0 if [[ "$IMPORT_INCLUDE_MAIL" == "true" ]]; then # The authority: whatever the classifier above decided, no symlink, no @@ -185,6 +202,9 @@ fi if [[ $TAR_RC -ne 0 ]]; then fail_extract "$TAR_RC" "tar failed (rc=$TAR_RC)$( [[ $VERIFY_RC -ne 0 ]] && echo "; mail-tree verify also failed (rc=$VERIFY_RC)" ); aborting" fi +if [[ $STRIP_RC -ne 0 ]]; then + fail_extract 4 "post-extract symlink check failed (rc=$STRIP_RC); aborting" +fi if [[ $VERIFY_RC -ne 0 ]]; then fail_extract 4 "extracted mail tree failed the link/readability check (see above); aborting — the same backup may import with 'Import without email'" fi diff --git a/scripts/lib/scan-symlinks.php b/scripts/lib/scan-symlinks.php index 0b2587c..21adeaf 100644 --- a/scripts/lib/scan-symlinks.php +++ b/scripts/lib/scan-symlinks.php @@ -92,10 +92,13 @@ $dangerousPrefixes = [ /** * Lexical resolution of a relative link from its own directory in the - * archive: a reason when it reaches/climbs above the archive root, or when a - * link under /homedir/ resolves outside that homedir; NULL otherwise. + * archive: a reason when it reaches/climbs above the archive root, when a + * link under /homedir/ resolves outside that homedir, or when any + * component on the way is itself a symlink entry ($linkSet: normalized + * archive path => true); NULL otherwise. extract.sh's post-extract + * strip-escaping-links.py (realpath of every link) is the authority. */ -function relative_symlink_escape(string $archivePath, string $target): ?string { +function relative_symlink_escape(string $archivePath, string $target, array $linkSet = []): ?string { $ap = preg_replace('#^(\./)+#', '', $archivePath); $dir = array_values(array_filter(explode('/', (string) $ap), fn($x) => $x !== '' && $x !== '.')); array_pop($dir); @@ -109,6 +112,10 @@ function relative_symlink_escape(string $archivePath, string $target): ?string { continue; } $stack[] = $seg; + // No chaining: a component that is itself a symlink entry in this + // archive would be expanded by the kernel, which this lexical walk + // cannot model (`a -> b/..` with `b -> x/y/z`). + if (isset($linkSet[implode('/', $stack)])) return 'passes through another symlink in the archive'; } if ($home !== null && (count($stack) < 2 || $stack[0] !== $home[0] || $stack[1] !== 'homedir')) { return 'escapes the account homedir'; @@ -126,6 +133,9 @@ if (!$fh) { exit(2); } +// Pass 1: every symlink entry (the chain rule needs the whole set). +$links = []; +$linkSet = []; while (($line = fgets($fh)) !== false) { if ($line === '' || $line[0] !== 'l') continue; $arrow = strpos($line, ' -> '); @@ -134,15 +144,20 @@ while (($line = fgets($fh)) !== false) { $right = rtrim(substr($line, $arrow + 4), "\r\n"); $parts = preg_split('/\s+/', $left, 6); if (count($parts) < 6) continue; - $archivePath = $parts[5]; - $target = $right; + $links[] = [$parts[5], $right]; + $linkSet[trim((string) preg_replace('#^(\./)+#', '', $parts[5]), '/')] = true; +} +pclose($fh); + +// Pass 2: classify. +foreach ($links as [$archivePath, $target]) { // N2-1: a RELATIVE target that climbs out of the archive (or out of // the account's homedir/) is DANGEROUS (stripped), not safe: GNU tar // extracts `cpmove-u/sds -> ../../../../root/.my.cnf` as-is. Same rule // as WHP's CpanelBackupImporter::relativeSymlinkEscape(). if ($target !== '' && $target[0] !== '/') { - $escape = relative_symlink_escape($archivePath, $target); + $escape = relative_symlink_escape($archivePath, $target, $linkSet); if ($escape !== null) { $findings[] = [ 'type' => 'DANGEROUS', @@ -212,7 +227,6 @@ while (($line = fgets($fh)) !== false) { 'reason' => $reason, ]; } -pclose($fh); $dangerousCount = count(array_filter($findings, fn($f) => $f['type'] === 'DANGEROUS')); $uncertainCount = count(array_filter($findings, fn($f) => $f['type'] === 'UNCERTAIN')); diff --git a/scripts/lib/strip-escaping-links.py b/scripts/lib/strip-escaping-links.py new file mode 100644 index 0000000..ffda4c0 --- /dev/null +++ b/scripts/lib/strip-escaping-links.py @@ -0,0 +1,102 @@ +#!/usr/bin/python3 -I +# strip-escaping-links.py — post-extract symlink authority (N2-1). +# +# python3 -I strip-escaping-links.py [username ...] +# +# The pre-extract classifier (scan-symlinks.php) resolves a relative target +# LEXICALLY, so a chain through another link (`a -> b/..` with `b -> x/y/z`, +# or `l1 -> l2/../../..`) can slip past it. This pass is the authority: it +# lstat-walks the WHOLE extracted tree (never following a link) and asks the +# kernel (realpath strict) where every symlink really resolves. A link is +# removed and recorded as a `stripped_escaping_symlink` action when: +# - it resolves outside , or to itself; +# - it is dangling (strict resolution fails), or loops. +# Exception: an ABSOLUTE target under /home[N]// for one of the +# given usernames (cPanel's own-account links, e.g. wp-content -> +# /home//shared). Those cannot resolve inside this sandbox by design, and +# WHP resolves them only inside the customer's own site container; the +# pre-extract scan calls them BENIGN_USER_INTERNAL. Every other absolute +# target either dangles here or resolves into the container's own /etc, +# /usr, ... — both outside — so it is stripped. +# +# Repeats until a pass strips nothing (removing one link can leave another +# that resolved through it dangling). Any walk error fails closed (exit 1). +# stdout: number of links stripped. Exit 0 ok, 1 error, 2 usage. + +import json +import os +import re +import stat +import sys + + +def main(argv): + if len(argv) < 3: + sys.stderr.write("usage: strip-escaping-links.py [username ...]\n") + return 2 + dest = os.path.realpath(argv[1]) + actions_path = argv[2] + users = [u for u in argv[3:] if u] + user_res = [re.compile(r'\A/home\d*/' + re.escape(u) + r'(/|\Z)') for u in users] + if not os.path.isdir(dest): + sys.stderr.write(f"strip-escaping-links: not a directory: {dest}\n") + return 1 + + def walk_error(err): + raise err + + stripped = [] + while True: + this_pass = 0 + for dirpath, dirnames, filenames in os.walk(dest, topdown=True, onerror=walk_error, followlinks=False): + for name in list(dirnames) + list(filenames): + p = os.path.join(dirpath, name) + st = os.lstat(p) + if not stat.S_ISLNK(st.st_mode): + continue + target = os.readlink(p) + if target.startswith('/') and any(r.match(target) for r in user_res): + continue + reason = None + try: + real = os.path.realpath(p, strict=True) + if not real.startswith(dest + '/'): + reason = 'resolves outside the extract' + except OSError: + reason = 'dangling or looping' + if reason is None: + continue + os.unlink(p) + if name in dirnames: + dirnames.remove(name) + stripped.append({ + 'action': 'stripped_escaping_symlink', + 'archive_path': os.path.relpath(p, dest), + 'target': target, + 'reason': reason, + }) + sys.stderr.write(f" STRIP {os.path.relpath(p, dest)} -> {target!r} ({reason})\n") + this_pass += 1 + if this_pass == 0: + break + + if stripped: + try: + with open(actions_path) as fh: + doc = json.load(fh) + except (OSError, ValueError): + doc = {'actions': []} + doc.setdefault('actions', []).extend(stripped) + doc['count'] = len(doc['actions']) + with open(actions_path, 'w') as fh: + json.dump(doc, fh, indent=2) + print(len(stripped)) + return 0 + + +if __name__ == '__main__': + try: + sys.exit(main(sys.argv)) + except OSError as e: + sys.stderr.write(f"strip-escaping-links: walk failed: {e}\n") + sys.exit(1) diff --git a/tests/build-fixtures.sh b/tests/build-fixtures.sh index 5ffa644..ac91f86 100755 --- a/tests/build-fixtures.sh +++ b/tests/build-fixtures.sh @@ -136,6 +136,23 @@ tar -C "$RELESC_TMP" -czf "$FIXTURES_DIR/cpmove-relesc.tar.gz" "cpmove-$USER" rm -rf "$RELESC_TMP" echo "wrote $FIXTURES_DIR/cpmove-relesc.tar.gz" +# ---- cpmove-chain.tar.gz (N2-1 review: symlink chains) ---------------------- +# Each link alone resolves lexically inside the archive; through the other +# link the kernel resolves it ABOVE the extract: +# d1/d2/b -> ../.. (= cpmove-$USER, fine on its own) +# d1/d2/a -> b/../.. (b/.. = extract root, /.. = its parent) +# d1/d2/l2 -> ../.. +# d1/d2/l1 -> l2/../../.. +CHAIN_TMP="$(mktemp -d)" +build_common_tree "$CHAIN_TMP" +C="$CHAIN_TMP/cpmove-$USER/d1/d2" +mkdir -p "$C" +ln -s "../.." "$C/b"; ln -s "b/../.." "$C/a" +ln -s "../.." "$C/l2"; ln -s "l2/../../.." "$C/l1" +tar -C "$CHAIN_TMP" -czf "$FIXTURES_DIR/cpmove-chain.tar.gz" "cpmove-$USER" +rm -rf "$CHAIN_TMP" +echo "wrote $FIXTURES_DIR/cpmove-chain.tar.gz" + # ---- cpmove-mail.tar.gz --------------------------------------------------- MAIL_TMP="$(mktemp -d)" diff --git a/tests/run-tests.sh b/tests/run-tests.sh index b4b1790..b32e369 100755 --- a/tests/run-tests.sh +++ b/tests/run-tests.sh @@ -77,6 +77,42 @@ check "relesc: dump.sql link stripped from the extract" "[[ ! -L '$OUT/$U/homedi check "relesc: homedir/www link kept" "[[ -L '$OUT/$U/homedir/www' ]]" check "relesc: both strips recorded" "grep -q '$U/sds' '$WORK/ex-relesc.actions.json' && grep -q 'public_html/dump.sql' '$WORK/ex-relesc.actions.json'" +echo "N2-1 review: symlink chains (cpmove-chain)" +cp "$FIX/cpmove-chain.tar.gz" "$WORK/" +php "$REPO/scripts/lib/scan-symlinks.php" --tarball "$WORK/cpmove-chain.tar.gz" --report "$WORK/relesc-scan.json" --username testuser 2>/dev/null +check "chain: d1/d2/a -> b/../.. (through link b) is DANGEROUS" "[[ \$(relesc_type '$U/d1/d2/a') == DANGEROUS ]]" +check "chain: d1/d2/l1 -> l2/../../.. (through link l2) is DANGEROUS" "[[ \$(relesc_type '$U/d1/d2/l1') == DANGEROUS ]]" +check "chain: d1/d2/b -> ../.. alone is not a finding" "[[ \$(relesc_type '$U/d1/d2/b') == NONE ]]" +OUT="$WORK/ex-chain" +env -u IMPORT_INCLUDE_MAIL bash "$REPO/scripts/extract.sh" "$WORK/cpmove-chain.tar.gz" "$OUT" testuser "$WORK/ex-chain.actions.json" >"$WORK/ex-chain.log" 2>&1 +check "chain: a and l1 absent from the extract" "[[ ! -L '$OUT/$U/d1/d2/a' && ! -L '$OUT/$U/d1/d2/l1' ]]" +check "chain: b and l2 kept" "[[ -L '$OUT/$U/d1/d2/b' && -L '$OUT/$U/d1/d2/l2' ]]" + +echo "N2-1 review: post-extract authority strip-escaping-links.py (links the classifier never saw)" +P="$WORK/postx" +rm -rf "$P"; mkdir -p "$P/$U/d1/d2" "$P/$U/homedir/public_html/inner" +echo x > "$P/$U/homedir/public_html/inner/f" +ln -s "../.." "$P/$U/d1/d2/b"; ln -s "b/../.." "$P/$U/d1/d2/a" +ln -s "../.." "$P/$U/d1/d2/l2"; ln -s "l2/../../.." "$P/$U/d1/d2/l1" +ln -s "/etc/passwd" "$P/$U/homedir/abs-etc" +ln -s "nonexistent" "$P/$U/homedir/public_html/x.php.bak.cpanel-import" +ln -s "../../../../../../victim/shell.php" "$P/$U/homedir/public_html/y.bak" +ln -s "loop2" "$P/$U/homedir/loop1"; ln -s "loop1" "$P/$U/homedir/loop2" +ln -s "../.." "$P/$U/top-link" +ln -s "inner/f" "$P/$U/homedir/public_html/ok-file" +ln -s "public_html" "$P/$U/homedir/www" +ln -s "/home/testuser/public_html/inner" "$P/$U/homedir/own-abs" +echo '{"actions":[{"action":"pre"}]}' > "$WORK/postx.json" +n=$(python3 -I "$REPO/scripts/lib/strip-escaping-links.py" "$P" "$WORK/postx.json" testuser 2>/dev/null) +for gone in d1/d2/a d1/d2/l1 homedir/abs-etc homedir/public_html/x.php.bak.cpanel-import homedir/public_html/y.bak homedir/loop1 homedir/loop2 top-link; do + check "post-extract: $gone stripped" "[[ ! -L '$P/$U/$gone' ]]" +done +for kept in d1/d2/b d1/d2/l2 homedir/public_html/ok-file homedir/www homedir/own-abs; do + check "post-extract: $kept kept" "[[ -L '$P/$U/$kept' ]]" +done +check "post-extract: 8 links stripped (got $n)" "[[ '$n' == 8 ]]" +check "post-extract: strips appended as stripped_escaping_symlink actions" "grep -q '\"pre\"' '$WORK/postx.json' && [[ \$(grep -c stripped_escaping_symlink '$WORK/postx.json') == 8 ]]" + echo "hard link crossing OUT of the mail tree (cpmove-mail-xlink)" cp "$FIX/cpmove-mail-xlink.tar.gz" "$WORK/" OUT="$WORK/ex-xlink"