hermes-admin/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 32 Packages Projects Releases Wiki Activity

refactor(pm): route .deb payloads through the one tar extractor

Browse Source 
DebPackage kept its own 116-line member walker next to
pm.store.extract_tar. Both enforced the same containment rules, so the
.deb data.tar now goes through extract_tar (which accepts an open
stream) and FilterError maps to InstallError. Link fallbacks on hosts
without symlink support come from tarfile's own copy fallback, which
also resolves chained aliases. The Termux mode normalization stays in
DebPackage, since it is .deb policy and not extraction.

The per-member FilterError/OSError swallow in pm/packages.py was already
removed; Git's MSYS /proc link exemption is unchanged.
This commit is contained in:
ethernet
2026-09-24 13:55:15 -04:00
parent 477565073a
commit 6dc1288dd1
3 changed files with 24 additions and 95 deletions
Download Patch File Download Diff File Expand all files Collapse all files

105
pm/package.py
View File

@@ -172,8 +172,8 @@ class DebPackage(Package):
.deb (Package + Version) must match the pin. Digest verification of
the downloaded bytes happens in the store, as for every package.
unpack() is the hardened extractor: traversal, symlink-target, and
member-type checks, shaped after the established safe-extract rules
unpack() pulls data.tar out of the ar container and hands it to
pm.store.extract_tar, the one containment policy for every PM tarball
(symlinks allowed with in-root targets; devices/fifos refused).
"""
@@ -210,98 +210,21 @@ class DebPackage(Package):
offset = start + size + (size % 2)
if payload is None:
raise InstallError(self.name, f"no data.tar member in {archive.name}")
self._safe_untar(payload, staged)
self._untar_payload(payload, staged)
def _safe_untar(self, payload: bytes, staged: Path) -> None:
def _untar_payload(self, payload: bytes, staged: Path) -> None:
import io
import posixpath
import shutil
import stat as stat_mod
import tarfile
from pm.store import extract_tar
try:
extract_tar(io.BytesIO(payload), staged)
except tarfile.FilterError as exc:
member = exc.tarinfo.name if exc.tarinfo is not None else "?"
raise InstallError(self.name, f"unsafe member {member!r}: {exc}") from exc
real_staged = os.path.realpath(staged)
def _contained(p: Path) -> bool:
"""True when p's REAL location (following any planted symlink
ancestors) stays inside the staged tree."""
real = os.path.realpath(p)
return real == real_staged or real.startswith(real_staged + os.sep)
deferred_links = []
with tarfile.open(fileobj=io.BytesIO(payload)) as tf:
for member in tf.getmembers():
path = PurePosixPath(member.name)
parts = tuple(q for q in path.parts if q not in ("", "."))
if path.is_absolute() or ".." in parts:
raise InstallError(self.name, f"unsafe member path {member.name!r}")
if not parts:
continue # the "./" root member
target = staged.joinpath(*parts)
if not _contained(target):
raise InstallError(self.name, f"member escapes staged tree: {member.name}")
if member.isdir():
target.mkdir(parents=True, exist_ok=True)
continue
if member.issym():
if PurePosixPath(member.linkname).is_absolute() or Path(member.linkname).is_absolute():
raise InstallError(
self.name,
f"absolute symlink target {member.linkname!r} "
f"in {member.name}",
)
link_dir = "/".join(parts[:-1])
resolved = posixpath.normpath(posixpath.join(link_dir, member.linkname))
if resolved == ".." or resolved.startswith("../"):
raise InstallError(self.name, f"symlink escapes root: {member.name}")
if not _contained(target.parent / member.linkname):
raise InstallError(self.name, f"symlink escapes staged tree: {member.name}")
target.parent.mkdir(parents=True, exist_ok=True)
if target.exists() or target.is_symlink():
target.unlink()
try:
target.symlink_to(member.linkname)
except OSError:
resolved_path = staged.joinpath(*resolved.split("/"))
if resolved_path.is_file():
shutil.copy2(resolved_path, target)
else:
deferred_links.append((member.linkname, target, resolved_path))
continue
if not member.isfile():
raise InstallError(self.name, f"unsupported member type: {member.name}")
target.parent.mkdir(parents=True, exist_ok=True)
if not _contained(target.parent):
raise InstallError(
self.name,
f"member {member.name} resolves outside the staged tree "
"through a symlinked ancestor",
)
extracted = tf.extractfile(member)
if extracted is None:
raise InstallError(self.name, f"cannot read member {member.name}")
with extracted, open(target, "wb") as dst:
shutil.copyfileobj(extracted, dst)
try:
target.chmod(member.mode & 0o777)
except OSError:
pass
while deferred_links:
pending = []
for linkname, target, resolved_path in deferred_links:
if not _contained(resolved_path):
raise InstallError(
self.name,
f"symlink {target.name} resolves outside the staged tree",
)
if target.exists() or target.is_symlink():
continue
if resolved_path.is_file():
shutil.copy2(resolved_path, target)
else:
pending.append((linkname, target, resolved_path))
if len(pending) == len(deferred_links):
break
deferred_links = pending
# Termux debs carry owner-only modes across the whole tree (700 on
# binaries n libs, 600 on stdlib .py files) -- postinst would
# normalize on a real phone, but pm extracts without postinst, and
@@ -321,13 +244,15 @@ class DebPackage(Package):
continue
if stat_mod.S_ISLNK(mode):
continue
if not _contained(f):
if not os.path.realpath(f).startswith(real_staged + os.sep):
continue
wanted = 0o644 | (0o111 if mode & 0o111 else 0)
try:
f.chmod(wanted)
except OSError:
pass
# Best effort by design: a file we cannot chmod keeps its
# extracted mode, and verify() still judges the tree.
continue
def verify(self, entry: Path, target: str) -> str:
"""'' when the staged tree is plausible on target: the expected

10
pm/store.py
View File

@@ -12,6 +12,7 @@ import tempfile
import time
from contextlib import contextmanager
from pathlib import Path
from typing import IO
from pm.filesystem import is_junction
@@ -157,8 +158,10 @@ def _tar_filter(member, dest: str):
return member.replace(deep=False, uid=None, gid=None, uname=None, gname=None, mode=None)
return tarfile.data_filter(member, dest)
def extract_tar(archive: Path, dest: Path, *, git_msys: bool = False) -> None:
"""Extract a tarball with one containment policy for PM's tar consumers.
def extract_tar(archive: Path | IO[bytes], dest: Path, *, git_msys: bool = False) -> None:
"""Extract a tarball (a path, or an open stream such as a .deb's data.tar)
with the one containment policy every PM tar consumer shares. Unsafe
members raise tarfile.FilterError.
MSYS Git ships dev/fd links and etc/mtab into /proc; those aren't usable
on Windows. Skip only those known links, never a filter error or failed file write.
@@ -167,7 +170,8 @@ def extract_tar(archive: Path, dest: Path, *, git_msys: bool = False) -> None:
dest.mkdir(parents=True, exist_ok=True)
real_dest = os.path.realpath(dest)
with tarfile.open(archive) as tf:
opened = tarfile.open(archive) if isinstance(archive, (str, os.PathLike)) else tarfile.open(fileobj=archive)
with opened as tf:
if git_msys:
members = (m for m in tf if not (m.issym() and (
(m.name.lstrip("./").startswith("dev/") and m.linkname.startswith("/proc/"))

4
tests/pm/test_deb_safety.py
View File

@@ -1,4 +1,4 @@
"""DebPackage._safe_untar containment regressions.
"""DebPackage.unpack containment regressions (via pm.store.extract_tar).
A safe extractor must forbid ANY write or chmod outside the staged tree.
The malicious .deb fixtures are real ar+tars built in a temp sandbox and
@@ -57,7 +57,7 @@ def test_chained_aliases_survive_without_symlink_support(tmp_path, monkeypatch):
def unavailable(*args, **kwargs):
raise OSError("symlinks unavailable")
monkeypatch.setattr(Path, "symlink_to", unavailable)
monkeypatch.setattr(os, "symlink", unavailable)
deb = tmp_path / "aliases.deb"
lib = "data/data/com.termux/files/usr/lib"
_build_deb(deb, [