From 52b1e656c35592651819341ef0cd9df54890bd52 Mon Sep 17 00:00:00 2001 From: Josh Knapp Date: Tue, 20 Oct 2026 09:13:26 -0700 Subject: [PATCH] extract: refuse parser-differential archives; post-extract link check is the authority Security review of 4788ca4 (inline python link classifier): 1. Names like cpmove-x/./homedir/mail/..., homedir//mail, homedir/../homedir or /cpmove-x/... were matched differently by the classifier than by tar, so a mail-tree symlink could escape the strip (measured: the dot, dslash and abs fixtures extracted with exit 0 on 4788ca4). 2. A newline in a member name split one --exclude-from pattern in two. Moved the policy to scripts/lib/mail-tree-links.py: - scan (pre-extract, an optimisation): REFUSES the archive (exit 3) on any control character in a member name or link target, or any non-canonical member name / hard-link target (absolute, '..', or normpath() differing after one leading './'). Symlink targets are not canonical-checked: ordinary cPanel links (../public_html) are legitimate outside the mail tree, and every symlink inside it is stripped anyway. - verify (post-extract, the authority): lstat-walks each mail tree; exit 4 on any symlink, or any regular file whose st_nlink exceeds its names inside that same mail tree (a link leaving it). In-tree Dovecot hard-link copies (maildir_copy_with_hardlinks) are kept; a blanket nlink>1 refusal would drop real mail. - Fixtures: cpmove-bad-{dot,dslash,dotdot,abs,newline}; verify tests for symlinks and hard links into/out of/between mail trees surviving the pre-filter. 14 of the new checks fail on 4788ca4. Only in IMPORT_INCLUDE_MAIL=true mode; the default path is unchanged. Co-Authored-By: Claude Opus 5.5 --- scripts/extract.sh | 88 +++---------------- scripts/lib/mail-tree-links.py | 149 +++++++++++++++++++++++++++++++++ tests/build-fixtures.sh | 31 +++++++ tests/run-tests.sh | 29 +++++++ 4 files changed, 221 insertions(+), 76 deletions(-) create mode 100644 scripts/lib/mail-tree-links.py diff --git a/scripts/extract.sh b/scripts/extract.sh index 8e344b9..040a3b7 100755 --- a/scripts/extract.sh +++ b/scripts/extract.sh @@ -106,72 +106,14 @@ fi # lists every entry in the tarball, mail included. if [[ "$IMPORT_INCLUDE_MAIL" == "true" ]]; then log "including mail tree (cpmove-*/homedir/mail) — IMPORT_INCLUDE_MAIL=true" - # A Maildir has no legitimate symlinks. scan-symlinks.php only strips - # DANGEROUS (absolute system-path) targets, so a relative symlink - # (../../../../etc/shadow, ../other-domain/box) would survive into the - # extract. Strip EVERY symlink entry under the mail tree. Hard links - # are different: Dovecot copies a message between folders as a hard - # link (maildir_copy_with_hardlinks, default yes), so a link whose name - # AND target are both inside the same account's mail tree is real mail - # and is kept. Any hard link crossing the mail-tree boundary (either - # direction) or with a ".." in its target is stripped. Read from the - # archive's own member list (python tarfile: exact names and types, no - # `tar -tv` text parsing); recorded as actions like the DANGEROUS - # strips; escaped for tar's glob-style --exclude-from. - MAIL_LINKS=$(python3 - "$TARBALL" "$EXCLUDES_FILE" "$ACTIONS_OUT" <<'PY' -import json, re, sys, tarfile -tar_path, excl_path, actions_path = sys.argv[1], sys.argv[2], sys.argv[3] -mail = re.compile(r'^(cpmove-[^/]+)/homedir/mail(/|$)') -def norm(n): - while n.startswith('./'): - n = n[2:] - return n.lstrip('/') -def mail_owner(n): - """The cpmove- whose mail tree holds n, else None ('..' never counts).""" - if '..' in n.split('/'): - return None - m = mail.match(n) - return m.group(1) if m else None -# Entries the DANGEROUS pass already strips are not listed twice. -try: - with open(actions_path) as ah: - already = {a.get('path') for a in json.load(ah).get('actions', [])} -except Exception: - already = set() -found = [] -with tarfile.open(tar_path, 'r|*') as tf: - for m in tf: - name = norm(m.name) - if m.issym() and mail.match(name): - found.append((m.name, 'symlink', m.linkname)) - elif m.islnk(): - target = norm(m.linkname) - in_name, in_target = mail_owner(name), mail_owner(target) - crosses = (in_name or mail.match(name)) or (in_target or mail.match(target)) - if crosses and not (in_name is not None and in_name == in_target): - found.append((m.name, 'hardlink', m.linkname)) -found = [f for f in found if f[0] not in already] -with open(excl_path, 'a') as eh: - for name, _, _ in found: - eh.write(re.sub(r'([*?\[\\])', r'\\\1', name) + '\n') -try: - with open(actions_path) as ah: - doc = json.load(ah) -except Exception: - doc = {'actions': [], 'count': 0} -doc['actions'] += [ - {'action': 'stripped_mail_link', 'path': name, 'target': target, - 'reason': f'{kind} inside the mail tree (Maildirs have no legitimate links)'} - for name, kind, target in found -] -doc['count'] = len(doc['actions']) -with open(actions_path, 'w') as ah: - json.dump(doc, ah, indent=2) -for name, kind, target in found: - sys.stderr.write(f' STRIP {kind}: {name} -> {target}\n') -print(len(found)) -PY -) || { log "mail-tree link scan failed; aborting"; exit 3; } + # Link policy for the mail tree (scripts/lib/mail-tree-links.py): every + # symlink in it, and every hard link crossing its boundary, is excluded + # here; Dovecot's in-tree hard-link copies are kept. The archive is + # refused outright if any member name could be read differently by the + # classifier and by tar (control characters, non-canonical paths). + # scan-symlinks.php alone only strips DANGEROUS absolute targets. + MAIL_LINKS=$(python3 -I "$SCRIPT_DIR/lib/mail-tree-links.py" scan "$TARBALL" "$EXCLUDES_FILE" "$ACTIONS_OUT") \ + || { log "mail-tree link scan refused the archive (see above); aborting"; exit 3; } log "stripped $MAIL_LINKS symlink / boundary-crossing hard-link entr(y/ies) in the mail tree" else { @@ -211,16 +153,10 @@ tar --no-same-owner --no-same-permissions --no-overwrite-dir \ $TAR_FLAGS "$TARBALL" if [[ "$IMPORT_INCLUDE_MAIL" == "true" ]]; then - # Belt: no symlink may remain under the mail tree (the excludes above - # should already guarantee it). Remove and log each one. - LEFT=0 - while IFS= read -r -d '' p; do - log " REMOVE symlink left in the mail tree after extract: ${p#"$DEST"/}" - rm -f -- "$p" - LEFT=$((LEFT + 1)) - done < <(find "$DEST" -mindepth 3 -maxdepth 3 -path "$DEST/cpmove-*/homedir/mail" -prune \ - -exec find {} -type l -print0 \;) - [[ $LEFT -eq 0 ]] || log "removed $LEFT symlink(s) from the mail tree after extract" + # The authority: whatever the classifier above decided, no symlink and + # no hard link leaving the mail tree may exist in the extract. + python3 -I "$SCRIPT_DIR/lib/mail-tree-links.py" verify "$DEST" \ + || { log "extracted mail tree contains links (see above); aborting"; exit 4; } fi log "extracted OK ($(find "$DEST" -type f | wc -l) files; $DANGEROUS_COUNT symlinks stripped)" diff --git a/scripts/lib/mail-tree-links.py b/scripts/lib/mail-tree-links.py new file mode 100644 index 0000000..13144b9 --- /dev/null +++ b/scripts/lib/mail-tree-links.py @@ -0,0 +1,149 @@ +#!/usr/bin/env python3 +""" +mail-tree-links.py — link policy for cpmove-*/homedir/mail when the mail +tree is extracted (IMPORT_INCLUDE_MAIL=true). Called by extract.sh. + +A Maildir has no legitimate symlinks. Hard links are different: Dovecot +copies a message between folders as a hard link (maildir_copy_with_hardlinks, +default yes), so a link whose name AND target are both inside the same +account's mail tree is real mail and is kept. Every other link touching a +mail tree is stripped (pre-extract) or fails the extract (post-extract). + + scan + Pre-extract classifier (an optimisation; `verify` is the authority). + Reads the archive's own member list with tarfile (exact names and + types, no `tar -tv` text parsing) and REFUSES the archive (exit 3) + when any member name, or hard-link target, could be read differently + by this classifier and by tar: + - a control character (\\x00-\\x1f, \\x7f) in any name or link + target (a newline would split one --exclude-from pattern in two); + - a non-canonical name: absolute, a '..' component, or + posixpath.normpath() differing from the name after stripping one + leading './' (e.g. 'x/./homedir', 'x/homedir//mail'). + Symlink TARGETS are not canonical-checked: cPanel archives carry + ordinary relative links (homedir/access-logs, about -> ../public_html) + outside the mail tree, and every symlink inside it is stripped anyway. + Then appends a glob-escaped --exclude-from line for every symlink under + a mail tree and every hard link that crosses a mail-tree boundary, and + records them as 'stripped_mail_link' actions. Prints the count. + + verify + Post-extract authority. lstat-walks every cpmove-*/homedir/mail under + (never following links) and exits 4 when it finds a symlink, or + a regular file whose st_nlink is larger than the number of its names + inside that same mail tree (some other name exists outside it). +""" +import json +import os +import posixpath +import re +import stat +import sys +import tarfile + +MAIL = re.compile(r'^(cpmove-[^/]+)/homedir/mail(/|$)') +CTRL = re.compile(r'[\x00-\x1f\x7f]') + + +def refuse(code, msg): + sys.stderr.write(f'mail-tree-links: REFUSING archive: {msg}\n') + sys.exit(code) + + +def canonical(raw): + """The name tar will use, or None when it is not canonical.""" + n = raw[2:] if raw.startswith('./') else raw + if n == '' or n.startswith('/') or '..' in n.split('/'): + return None + return n if posixpath.normpath(n) == n else None + + +def owner(n): + m = MAIL.match(n) + return m.group(1) if m else None + + +def scan(tar_path, excl_path, actions_path): + try: + with open(actions_path) as ah: + doc = json.load(ah) + except Exception: + doc = {'actions': [], 'count': 0} + already = {a.get('path') for a in doc.get('actions', [])} + found = [] + with tarfile.open(tar_path, 'r|*') as tf: + for m in tf: + for label, value in (('name', m.name), ('link target', m.linkname)): + if CTRL.search(value or ''): + refuse(3, f'member {label} contains a control character: {value!r}') + name = canonical(m.name) + if name is None: + refuse(3, f'non-canonical member name: {m.name!r}') + if m.issym(): + if owner(name): + found.append((m.name, 'symlink', m.linkname)) + elif m.islnk(): + target = canonical(m.linkname) + if target is None: + refuse(3, f'non-canonical hard-link target: {m.name!r} -> {m.linkname!r}') + a, b = owner(name), owner(target) + if (a or b) and a != b: + found.append((m.name, 'hardlink', m.linkname)) + found = [f for f in found if f[0] not in already] + with open(excl_path, 'a') as eh: + for name, _, _ in found: + eh.write(re.sub(r'([*?\[\\])', r'\\\1', name) + '\n') + doc['actions'] = doc.get('actions', []) + [ + {'action': 'stripped_mail_link', 'path': name, 'target': target, + 'reason': f'{kind} inside the mail tree (Maildirs have no legitimate links outside it)'} + for name, kind, target in found + ] + doc['count'] = len(doc['actions']) + with open(actions_path, 'w') as ah: + json.dump(doc, ah, indent=2) + for name, kind, target in found: + sys.stderr.write(f' STRIP {kind}: {name} -> {target}\n') + print(len(found)) + + +def verify(dest): + bad = [] + for top in sorted(os.listdir(dest)): + mail = os.path.join(dest, top, 'homedir', 'mail') + if not top.startswith('cpmove-'): + continue + try: + st = os.lstat(mail) + except FileNotFoundError: + continue + if stat.S_ISLNK(st.st_mode): + bad.append(f'symlink: {top}/homedir/mail') + continue + names = {} # (dev, ino) -> [count of names in this tree, st_nlink, a path] + for root, dirs, files in os.walk(mail, followlinks=False): + for e in dirs + files: + p = os.path.join(root, e) + s = os.lstat(p) + if stat.S_ISLNK(s.st_mode): + bad.append(f'symlink: {os.path.relpath(p, dest)}') + elif stat.S_ISREG(s.st_mode) and s.st_nlink > 1: + rec = names.setdefault((s.st_dev, s.st_ino), [0, s.st_nlink, p]) + rec[0] += 1 + for count, nlink, p in names.values(): + if nlink > count: + bad.append(f'hard link leaving the mail tree: {os.path.relpath(p, dest)} ' + f'(st_nlink {nlink}, {count} name(s) inside the mail tree)') + for b in bad: + sys.stderr.write(f'mail-tree-links: {b}\n') + if bad: + refuse(4, f'{len(bad)} link(s) in the extracted mail tree') + + +if __name__ == '__main__': + if len(sys.argv) == 5 and sys.argv[1] == 'scan': + scan(sys.argv[2], sys.argv[3], sys.argv[4]) + elif len(sys.argv) == 3 and sys.argv[1] == 'verify': + verify(sys.argv[2]) + else: + sys.stderr.write('usage: mail-tree-links.py scan | verify \n') + sys.exit(2) diff --git a/tests/build-fixtures.sh b/tests/build-fixtures.sh index 7c00708..76fde87 100755 --- a/tests/build-fixtures.sh +++ b/tests/build-fixtures.sh @@ -168,6 +168,37 @@ ln "$MROOT/mail/$DOMAIN/alice/.Archive/cur/1700000002.M3P1.host,S=41:2,S" \ tar -C "$MAIL_TMP" --sort=name -czf "$FIXTURES_DIR/cpmove-mail-xlink.tar.gz" "cpmove-$USER" echo "wrote $FIXTURES_DIR/cpmove-mail.tar.gz + cpmove-mail-xlink.tar.gz" +# ---- parser-differential fixtures (must be REFUSED with IMPORT_INCLUDE_MAIL=true) +# Written with python tarfile because GNU tar normalises such names on create. +# cpmove-bad-dot ./ component: cpmove-testuser/./homedir/mail/.../.x -> ../../../../etc +# cpmove-bad-dslash empty component: cpmove-testuser/homedir//mail/.../.x -> ../../../../etc +# cpmove-bad-dotdot '..' component: cpmove-testuser/homedir/../homedir/mail/.../.x -> ../../../../etc +# cpmove-bad-abs absolute name: /cpmove-testuser/homedir/mail/.../.x -> ../../../../etc +# cpmove-bad-newline '\n' in a name (would split one --exclude-from pattern in two) +python3 - "$FIXTURES_DIR" <<'PY' +import io, sys, tarfile +out = sys.argv[1] +def build(fname, members): + with tarfile.open(f'{out}/{fname}', 'w:gz', format=tarfile.PAX_FORMAT) as tf: + for d in ('cpmove-testuser', 'cpmove-testuser/homedir', 'cpmove-testuser/homedir/mail', + 'cpmove-testuser/homedir/mail/example.com', 'cpmove-testuser/homedir/mail/example.com/alice', + 'cpmove-testuser/homedir/mail/example.com/alice/cur'): + ti = tarfile.TarInfo(d); ti.type = tarfile.DIRTYPE; ti.mode = 0o755; tf.addfile(ti) + for name, kind, arg in members: + ti = tarfile.TarInfo(name); ti.mode = 0o644 + if kind == 'sym': + ti.type = tarfile.SYMTYPE; ti.linkname = arg; tf.addfile(ti) + else: + ti.size = len(arg); tf.addfile(ti, io.BytesIO(arg)) +mb = 'homedir/mail/example.com/alice' +build('cpmove-bad-dot.tar.gz', [(f'cpmove-testuser/./{mb}/.x', 'sym', '../../../../etc')]) +build('cpmove-bad-dslash.tar.gz', [(f'cpmove-testuser/homedir//mail/example.com/alice/.x', 'sym', '../../../../etc')]) +build('cpmove-bad-dotdot.tar.gz', [(f'cpmove-testuser/homedir/../{mb}/.x', 'sym', '../../../../etc')]) +build('cpmove-bad-abs.tar.gz', [(f'/cpmove-testuser/{mb}/.x', 'sym', '../../../../etc')]) +build('cpmove-bad-newline.tar.gz', [(f'cpmove-testuser/{mb}/cur/a\ncpmove-testuser/homedir/public_html', 'file', b'x')]) +PY +echo "wrote $FIXTURES_DIR/cpmove-bad-{dot,dslash,dotdot,abs,newline}.tar.gz" + echo "" echo "fixtures built:" ls -la "$FIXTURES_DIR" diff --git a/tests/run-tests.sh b/tests/run-tests.sh index faa9db8..8b44537 100755 --- a/tests/run-tests.sh +++ b/tests/run-tests.sh @@ -71,6 +71,35 @@ check "opt-in: the linked message itself still extracted" "[[ -f '$OUT/$U/homedi check "default mode: same archive fails closed (tar cannot link to the excluded mail member)" \ "! env -u IMPORT_INCLUDE_MAIL bash '$REPO/scripts/extract.sh' '$WORK/cpmove-mail-xlink.tar.gz' '$WORK/ex-xlink-default' testuser '$WORK/x.json' >/dev/null 2>&1" +echo "parser-differential archives are refused (IMPORT_INCLUDE_MAIL=true)" +for bad in dot dslash dotdot abs newline; do + cp "$FIX/cpmove-bad-$bad.tar.gz" "$WORK/" + OUT="$WORK/ex-bad-$bad" + rc=0 + IMPORT_INCLUDE_MAIL=true bash "$REPO/scripts/extract.sh" "$WORK/cpmove-bad-$bad.tar.gz" "$OUT" testuser "$WORK/bad-$bad.json" >"$WORK/bad-$bad.log" 2>&1 || rc=$? + check "bad-$bad: extract exits non-zero (rc=$rc)" "[[ $rc -ne 0 ]]" + check "bad-$bad: refusal logged" "grep -q 'REFUSING archive' '$WORK/bad-$bad.log'" + check "bad-$bad: nothing extracted" "[[ -z \$(find '$OUT' -mindepth 1 2>/dev/null) ]]" +done + +echo "post-extract authority: mail-tree-links.py verify" +V="$WORK/verify" +mkvt() { rm -rf "$V"; mkdir -p "$V/$U/homedir/mail/$D/a/cur" "$V/$U/homedir/mail/$D/a/.F/cur" "$V/$U/homedir/public_html" "$V/cpmove-other/homedir/mail/$D/b/cur"; echo m > "$V/$U/homedir/mail/$D/a/cur/1"; } +vrc() { local rc=0; python3 -I "$REPO/scripts/lib/mail-tree-links.py" verify "$V" >/dev/null 2>&1 || rc=$?; echo $rc; } +mkvt; check "verify: clean tree => 0" "[[ \$(vrc) == 0 ]]" +mkvt; ln "$V/$U/homedir/mail/$D/a/cur/1" "$V/$U/homedir/mail/$D/a/.F/cur/1" +check "verify: Dovecot hard link inside one mail tree => 0 (kept)" "[[ \$(vrc) == 0 ]]" +mkvt; ln -s ../../../../../etc "$V/$U/homedir/mail/$D/a/.evil" +check "verify: symlink in the mail tree (survived the pre-filter) => 4" "[[ \$(vrc) == 4 ]]" +mkvt; ln "$V/$U/homedir/mail/$D/a/cur/1" "$V/$U/homedir/public_html/out" +check "verify: hard link OUT of the mail tree (survived the pre-filter) => 4" "[[ \$(vrc) == 4 ]]" +mkvt; echo s > "$V/$U/homedir/shadow"; ln "$V/$U/homedir/shadow" "$V/$U/homedir/mail/$D/a/cur/in" +check "verify: hard link INTO the mail tree (survived the pre-filter) => 4" "[[ \$(vrc) == 4 ]]" +mkvt; ln "$V/$U/homedir/mail/$D/a/cur/1" "$V/cpmove-other/homedir/mail/$D/b/cur/1" +check "verify: hard link between two accounts' mail trees => 4" "[[ \$(vrc) == 4 ]]" +mkvt; rm -rf "$V/$U/homedir/mail"; ln -s /etc "$V/$U/homedir/mail" +check "verify: homedir/mail itself a symlink => 4" "[[ \$(vrc) == 4 ]]" + echo "extract.sh with IMPORT_INCLUDE_MAIL=1 (only \"true\" opts in)" OUT="$WORK/ex-one" IMPORT_INCLUDE_MAIL=1 bash "$REPO/scripts/extract.sh" "$MAILTAR" "$OUT" testuser "$WORK/ex-one.actions.json" >/dev/null 2>&1