extract: own-account links judged re-rooted, normalized, for the trusted username only (N2-1 review 2)
- strip-escaping-links.py: the /home[N]/<user>/ exemption ran on the RAW
target (`/home/u/../../etc/shadow` was exempt) and on usernames taken
from the archive's own top-dir names (an archive could name itself
cpmove-root). Now only the username WHP passes (extract.sh <username> =
IMPORT_USERNAME, validated) is trusted; the target must be normalized; and
it is RE-ROOTED at the link's <top>/homedir in the extract and kept only
if that resolves (realpath strict) inside it. No trusted name: nothing
exempt.
- scan-symlinks.php: same trusted-name rule; a non-normalized absolute
target is DANGEROUS outright (never prefix-compared).
- tests: /home/u/../../etc/shadow, /home/u/./x, re-rooted dangling,
re-rooted through an escaping link, another user's /home, an
archive-named user (cpmove-root) via extract.sh, no/invalid trusted name.
8 checks fail on 72b8fa9.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
72b8fa9d08
commit
36d4949d34
4 files changed
+109
-35
No files matched your search
+5
-8
@@ -179,15 +179,12 @@ tar --no-same-owner --no-same-permissions --no-overwrite-dir \
|
||||
# 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/<user>/ 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
|
||||
# $DEST itself, dangle or loop. A walk error fails the extract closed.
|
||||
# Only the TRUSTED username (<username>, the WHP account WHP passes) is
|
||||
# exempted, and only re-rooted inside the extract's homedir; names read from
|
||||
# the archive (top-dir names) are never trusted.
|
||||
STRIP_RC=0
|
||||
ESCAPING=$(python3 -I "$SCRIPT_DIR/lib/strip-escaping-links.py" "$DEST" "$ACTIONS_OUT" "${LINK_USERS[@]}") || STRIP_RC=$?
|
||||
ESCAPING=$(python3 -I "$SCRIPT_DIR/lib/strip-escaping-links.py" "$DEST" "$ACTIONS_OUT" "$USERNAME") || 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
|
||||
|
||||
@@ -124,7 +124,6 @@ function relative_symlink_escape(string $archivePath, string $target, array $lin
|
||||
}
|
||||
|
||||
$findings = [];
|
||||
$cpanelUsername = null;
|
||||
|
||||
$cmd = 'tar -tvf ' . escapeshellarg($tarPath) . ' 2>/dev/null';
|
||||
$fh = @popen($cmd, 'r');
|
||||
@@ -170,19 +169,18 @@ foreach ($links as [$archivePath, $target]) {
|
||||
}
|
||||
if ($target === '') continue;
|
||||
|
||||
if ($cpanelUsername === null) {
|
||||
if (preg_match('#^cpmove-([^/]+)/#', $archivePath, $m)) {
|
||||
$cpanelUsername = $m[1];
|
||||
}
|
||||
}
|
||||
|
||||
// (1) user-internal — accept symlinks pointing into the customer's
|
||||
// own /home/<user>/ tree. The panel rewrites these on extract.
|
||||
$userInternal = false;
|
||||
$usernames = [];
|
||||
if ($cpanelUsername !== null && $cpanelUsername !== '') $usernames[] = $cpanelUsername;
|
||||
if ($username !== '') $usernames[] = $username;
|
||||
foreach ($usernames as $u) {
|
||||
// N2-1 review: only the TRUSTED --username (the WHP account WHP passes);
|
||||
// a name read from the archive's top dir (cpmove-<name>) is never trusted.
|
||||
if (preg_match('/\A[a-z_][a-z0-9_-]{0,31}\z/', $username)) $usernames[] = $username;
|
||||
// N2-1 review: only a NORMALIZED target can be user-internal
|
||||
// (`/home/<u>/../../etc/shadow` is not).
|
||||
$normalized = !preg_match('#(^|/)\.{1,2}(/|$)|//#', $target);
|
||||
foreach ($normalized ? $usernames : [] as $u) {
|
||||
$prefix = '/home/' . $u . '/';
|
||||
if (strpos($target, $prefix) === 0 || $target === rtrim($prefix, '/')) {
|
||||
$userInternal = true;
|
||||
@@ -195,6 +193,19 @@ foreach ($links as [$archivePath, $target]) {
|
||||
}
|
||||
if ($userInternal) continue;
|
||||
|
||||
// N2-1 review: a non-normalized absolute target ('.', '..', '//') is
|
||||
// never compared by prefix (`/home/<u>/../../etc` would dodge /etc):
|
||||
// DANGEROUS outright.
|
||||
if (!$normalized) {
|
||||
$findings[] = [
|
||||
'type' => 'DANGEROUS',
|
||||
'archive_path' => $archivePath,
|
||||
'target' => $target,
|
||||
'reason' => "absolute target is not normalized ('.', '..' or empty component)",
|
||||
];
|
||||
continue;
|
||||
}
|
||||
|
||||
// (2) exact root.
|
||||
$type = null;
|
||||
$reason = '';
|
||||
|
||||
@@ -11,13 +11,17 @@
|
||||
# removed and recorded as a `stripped_escaping_symlink` action when:
|
||||
# - it resolves outside <dest>, or to <dest> itself;
|
||||
# - it is dangling (strict resolution fails), or loops.
|
||||
# Exception: an ABSOLUTE target under /home[N]/<username>/ for one of the
|
||||
# given usernames (cPanel's own-account links, e.g. wp-content ->
|
||||
# /home/<u>/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 <dest> — so it is stripped.
|
||||
# Own-account absolute links (cPanel's wp-content -> /home/<u>/shared and
|
||||
# the like) are judged RE-ROOTED, never against the container's own /home:
|
||||
# a target that is already normalized (no '.', '..' or empty component)
|
||||
# and lies under /home[N]/<user>/ for the TRUSTED username only (the WHP
|
||||
# account name WHP passes as extract.sh's <username>; names read from the
|
||||
# archive are never trusted) is mapped to <dest>/<top>/homedir/<rest>, where
|
||||
# <top> is the link's own top-level dir, and kept only if THAT resolves
|
||||
# (realpath strict) inside <top>/homedir. Every other absolute target either
|
||||
# dangles here or resolves into the container's own /etc, /usr, ... — both
|
||||
# outside <dest> — so it is stripped. A link to the extract itself is
|
||||
# stripped too.
|
||||
#
|
||||
# Repeats until a pass strips nothing (removing one link can leave another
|
||||
# that resolved through it dangling). Any walk error fails closed (exit 1).
|
||||
@@ -25,6 +29,7 @@
|
||||
|
||||
import json
|
||||
import os
|
||||
import posixpath
|
||||
import re
|
||||
import stat
|
||||
import sys
|
||||
@@ -32,12 +37,37 @@ import sys
|
||||
|
||||
def main(argv):
|
||||
if len(argv) < 3:
|
||||
sys.stderr.write("usage: strip-escaping-links.py <dest> <actions.json> [username ...]\n")
|
||||
sys.stderr.write("usage: strip-escaping-links.py <dest> <actions.json> [trusted-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]
|
||||
trusted = argv[3] if len(argv) > 3 else ''
|
||||
if not re.fullmatch(r'[a-z_][a-z0-9_-]{0,31}', trusted):
|
||||
trusted = '' # no (valid) trusted name: exempt nothing
|
||||
own_re = re.compile(r'\A/home\d*/' + re.escape(trusted) + r'(?:/(.*))?\Z') if trusted else None
|
||||
|
||||
def own_account_ok(link_path, target):
|
||||
"""True when an absolute own-account target, re-rooted at the link's
|
||||
<top>/homedir, resolves inside that homedir."""
|
||||
if own_re is None or posixpath.normpath(target) != target.rstrip('/') or '/../' in target + '/' or '/./' in target + '/':
|
||||
return False
|
||||
m = own_re.match(target.rstrip('/'))
|
||||
if not m:
|
||||
return False
|
||||
rel = os.path.relpath(link_path, dest).split(os.sep)
|
||||
if len(rel) < 2:
|
||||
return False
|
||||
home = os.path.join(dest, rel[0], 'homedir')
|
||||
try:
|
||||
hst = os.lstat(home)
|
||||
if not stat.S_ISDIR(hst.st_mode):
|
||||
return False
|
||||
home_real = os.path.realpath(home, strict=True)
|
||||
cand = os.path.join(home_real, m.group(1)) if m.group(1) else home_real
|
||||
real = os.path.realpath(cand, strict=True)
|
||||
except OSError:
|
||||
return False
|
||||
return real == home_real or real.startswith(home_real + '/')
|
||||
if not os.path.isdir(dest):
|
||||
sys.stderr.write(f"strip-escaping-links: not a directory: {dest}\n")
|
||||
return 1
|
||||
@@ -55,13 +85,16 @@ def main(argv):
|
||||
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
|
||||
if target.startswith('/') and own_re is not None and own_re.match(target.rstrip('/') or '/'):
|
||||
if own_account_ok(p, target):
|
||||
continue
|
||||
reason = 'own-account target does not resolve inside the extracted homedir'
|
||||
try:
|
||||
real = os.path.realpath(p, strict=True)
|
||||
if not real.startswith(dest + '/'):
|
||||
reason = 'resolves outside the extract'
|
||||
if reason is None:
|
||||
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:
|
||||
|
||||
Reference in new issue
Block a user