diff --git a/scripts/extract.sh b/scripts/extract.sh index 040a3b7..25ea716 100755 --- a/scripts/extract.sh +++ b/scripts/extract.sh @@ -36,6 +36,24 @@ log() { printf '[%s] extract: %s\n' "$(ts)" "$*"; } [[ -f "$TARBALL" ]] || { log "tarball not found: $TARBALL"; exit 2; } mkdir -p "$DEST" +# Any failure after this point removes the (partial) extract before exiting: +# it is the container's own scratch dir inside the bind-mounted output, and a +# half-extracted tree, or one the link check refused, must not be left +# behind for anything to read. Directories the archive made unreadable are +# opened up first (find -P never follows links; -type d skips them) so rm +# can descend. +fail_extract() { + local code="$1"; shift + log "$*" + cd / + if [[ -n "$DEST" && "$DEST" != "/" && -d "$DEST" ]]; then + find -P "$DEST" -type d ! -perm -u=rwx -exec chmod u+rwx {} \; 2>/dev/null || true + rm -rf -- "$DEST" || log "WARN: could not remove the failed extract $DEST" + log "removed the failed extract $DEST" + fi + exit "$code" +} + # --- pre-extract symlink scan --------------------------------------------- log "scanning tarball for dangerous symlinks (cpmove vector check)" @@ -44,9 +62,8 @@ if ! php "$SCRIPT_DIR/lib/scan-symlinks.php" \ --tarball "$TARBALL" \ --username "$USERNAME" \ --report "$SYMLINK_REPORT"; then - log "scan-symlinks.php exited with usage/IO error; aborting (this is not a sanitize-able state)" cat "$SYMLINK_REPORT" >&2 || true - exit 3 + fail_extract 3 "scan-symlinks.php exited with usage/IO error; aborting (this is not a sanitize-able state)" fi # --- compute exclude list from dangerous findings ------------------------- @@ -113,7 +130,7 @@ if [[ "$IMPORT_INCLUDE_MAIL" == "true" ]]; then # 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; } + || fail_extract 3 "mail-tree link scan refused the archive (see above); aborting" log "stripped $MAIL_LINKS symlink / boundary-crossing hard-link entr(y/ies) in the mail tree" else { @@ -148,15 +165,26 @@ log "extracting with hardened tar flags into $DEST" # match is effectively a literal-path skip. # --absolute-names is NOT used — leading / in a member name is stripped. cd "$DEST" +# tar's exit code is captured, not fatal on the spot: the mail-tree verify +# below must run on whatever tar did write, and a failed tar must still get +# its partial tree removed. +TAR_RC=0 tar --no-same-owner --no-same-permissions --no-overwrite-dir \ --exclude-from="$EXCLUDES_FILE" \ - $TAR_FLAGS "$TARBALL" + $TAR_FLAGS "$TARBALL" || TAR_RC=$? +VERIFY_RC=0 if [[ "$IMPORT_INCLUDE_MAIL" == "true" ]]; then - # 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; } + # The authority: whatever the classifier above decided, no symlink, no + # hard link leaving the mail tree, and no uninspectable directory may + # exist in the extract. Always runs, even after a failed tar. + python3 -I "$SCRIPT_DIR/lib/mail-tree-links.py" verify "$DEST" || VERIFY_RC=$? +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 [[ $VERIFY_RC -ne 0 ]]; then + fail_extract 4 "extracted mail tree failed the link/readability check (see above); aborting" 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 index 13144b9..2bfad8c 100644 --- a/scripts/lib/mail-tree-links.py +++ b/scripts/lib/mail-tree-links.py @@ -31,13 +31,19 @@ mail tree is stripped (pre-extract) or fails the extract (post-extract). 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). + inside that same mail tree (some other name exists outside it), or any + directory it cannot read+search (an uninspectable subtree fails). + + scan also cross-checks its member count against `tar -tf` and refuses on + any difference or stream error, so nothing GNU tar extracts goes + unclassified (verify remains the authority either way). """ import json import os import posixpath import re import stat +import subprocess import sys import tarfile @@ -71,24 +77,47 @@ def scan(tar_path, excl_path, actions_path): 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)) + seen = 0 + try: + tf = tarfile.open(tar_path, 'r|*') + except Exception as e: + refuse(3, f'archive unreadable by the classifier: {e}') + try: + with tf: + for m in tf: + seen += 1 + 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)) + except Exception as e: # TarError, EOFError, zlib.error ... (SystemExit passes through) + refuse(3, f'archive stream error after {seen} member(s): {e}') + # The classifier must have seen every member GNU tar will extract: a + # stream that stops early (zero block, truncation, a header tarfile + # skips) would leave members unclassified. Count GNU tar's own listing + # (escape quoting = one line per member, even with a newline in a name). + try: + lst = subprocess.run(['tar', '--quoting-style=escape', '-tf', tar_path], + stdout=subprocess.PIPE, stderr=subprocess.PIPE, check=False) + except OSError as e: + refuse(3, f'cannot list the archive with tar: {e}') + if lst.returncode != 0: + refuse(3, f'tar -tf failed (rc={lst.returncode}): {lst.stderr.decode(errors="replace").strip()[:300]}') + tar_count = lst.stdout.count(b'\n') + if tar_count != seen: + refuse(3, f'member count differs: classifier saw {seen}, tar lists {tar_count}') found = [f for f in found if f[0] not in already] with open(excl_path, 'a') as eh: for name, _, _ in found: @@ -108,7 +137,15 @@ def scan(tar_path, excl_path, actions_path): def verify(dest): bad = [] - for top in sorted(os.listdir(dest)): + + def walk_error(err): + bad.append(f'cannot inspect: {os.path.relpath(getattr(err, "filename", "") or dest, dest)} ({err.strerror})') + + try: + tops = sorted(os.listdir(dest)) + except OSError as e: + refuse(4, f'cannot list the extract: {e}') + for top in tops: mail = os.path.join(dest, top, 'homedir', 'mail') if not top.startswith('cpmove-'): continue @@ -119,13 +156,23 @@ def verify(dest): if stat.S_ISLNK(st.st_mode): bad.append(f'symlink: {top}/homedir/mail') continue + if stat.S_ISDIR(st.st_mode) and not os.access(mail, os.R_OK | os.X_OK): + bad.append(f'directory not readable+searchable: {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): + # Fail closed: os.walk silently skips what it can't read unless told. + for root, dirs, files in os.walk(mail, followlinks=False, onerror=walk_error): for e in dirs + files: p = os.path.join(root, e) - s = os.lstat(p) + try: + s = os.lstat(p) + except OSError as err: + walk_error(err) + continue if stat.S_ISLNK(s.st_mode): bad.append(f'symlink: {os.path.relpath(p, dest)}') + elif stat.S_ISDIR(s.st_mode) and not os.access(p, os.R_OK | os.X_OK): + bad.append(f'directory not readable+searchable: {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 @@ -136,7 +183,7 @@ def verify(dest): 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') + refuse(4, f'{len(bad)} problem(s) in the extracted mail tree (links or uninspectable directories)') if __name__ == '__main__': diff --git a/tests/build-fixtures.sh b/tests/build-fixtures.sh index 76fde87..115a805 100755 --- a/tests/build-fixtures.sh +++ b/tests/build-fixtures.sh @@ -188,6 +188,10 @@ def build(fname, members): ti = tarfile.TarInfo(name); ti.mode = 0o644 if kind == 'sym': ti.type = tarfile.SYMTYPE; ti.linkname = arg; tf.addfile(ti) + elif kind == 'hard': + ti.type = tarfile.LNKTYPE; ti.linkname = arg; tf.addfile(ti) + elif kind in ('dir', 'dir0'): + ti.type = tarfile.DIRTYPE; ti.mode = 0o755 if kind == 'dir' else 0; tf.addfile(ti) else: ti.size = len(arg); tf.addfile(ti, io.BytesIO(arg)) mb = 'homedir/mail/example.com/alice' @@ -196,8 +200,23 @@ build('cpmove-bad-dslash.tar.gz', [(f'cpmove-testuser/homedir//mail/example.com 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')]) +# A mail-tree directory the archive makes unreadable (mode 0000): verify +# cannot inspect it, so the extract must fail (and be removed). +build('cpmove-bad-unreadable.tar.gz', [(f'cpmove-testuser/{mb}/cur/m1', 'file', b'hello'), + (f'cpmove-testuser/{mb}/.Locked', 'dir0', None), + (f'cpmove-testuser/{mb}/.Locked/cur', 'dir0', None)]) +# Passes the classifier but GNU tar fails mid-way (hard link to a member that +# does not exist), after it already wrote the mail files: the partial tree +# must be removed. +build('cpmove-bad-tarfail.tar.gz', [(f'cpmove-testuser/{mb}/cur/m1', 'file', b'hello'), + ('cpmove-testuser/homedir/public_html', 'dir', None), + ('cpmove-testuser/homedir/public_html/x', 'hard', 'cpmove-testuser/homedir/nonexistent'), + (f'cpmove-testuser/{mb}/cur/m2', 'file', b'after the failure')]) PY -echo "wrote $FIXTURES_DIR/cpmove-bad-{dot,dslash,dotdot,abs,newline}.tar.gz" +# Truncated archive: the first half of a valid one. +head -c "$(( $(stat -c%s "$FIXTURES_DIR/cpmove-mail.tar.gz") / 2 ))" "$FIXTURES_DIR/cpmove-mail.tar.gz" \ + > "$FIXTURES_DIR/cpmove-bad-truncated.tar.gz" +echo "wrote $FIXTURES_DIR/cpmove-bad-{dot,dslash,dotdot,abs,newline,unreadable,tarfail,truncated}.tar.gz" echo "" echo "fixtures built:" diff --git a/tests/run-tests.sh b/tests/run-tests.sh index 8b44537..40836cb 100755 --- a/tests/run-tests.sh +++ b/tests/run-tests.sh @@ -79,9 +79,43 @@ for bad in dot dslash dotdot abs newline; do 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) ]]" + check "bad-$bad: extract dir removed" "[[ ! -e '$OUT' ]]" done +echo "failures after extraction starts remove the extract (IMPORT_INCLUDE_MAIL=true)" +for bad in unreadable tarfail truncated; 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: extract dir removed" "[[ ! -e '$OUT' ]]" +done +check "bad-unreadable: failed in verify (cannot inspect the 0000 dir)" "grep -q 'not readable+searchable\|cannot inspect' '$WORK/bad-unreadable.log'" +check "bad-tarfail: tar itself failed (rc logged), verify still ran" "grep -q 'tar failed (rc=' '$WORK/bad-tarfail.log'" +check "bad-truncated: refused by the classifier" "grep -q 'REFUSING archive' '$WORK/bad-truncated.log'" + +echo "classifier refuses when GNU tar lists a different member count" +FAKEBIN="$WORK/faketar" +mkdir -p "$FAKEBIN" +REALTAR="$(command -v tar)" +cat > "$FAKEBIN/tar" < "$WORK/cm.excl"; echo '{"actions":[]}' > "$WORK/cm.json" +rc=0 +PATH="$FAKEBIN:$PATH" python3 -I "$REPO/scripts/lib/mail-tree-links.py" scan "$MAILTAR" "$WORK/cm.excl" "$WORK/cm.json" >/dev/null 2>"$WORK/cm.log" || rc=$? +check "count mismatch: scan exits 3 (rc=$rc)" "[[ $rc -eq 3 ]]" +check "count mismatch: reason logged" "grep -q 'member count differs' '$WORK/cm.log'" +rc=0 +python3 -I "$REPO/scripts/lib/mail-tree-links.py" scan "$MAILTAR" "$WORK/cm.excl" "$WORK/cm.json" >/dev/null 2>&1 || rc=$? +check "control: real tar, same archive => scan exits 0" "[[ $rc -eq 0 ]]" + 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"; }