diff --git a/src/borg/archive.py b/src/borg/archive.py index 6e107896a7..37557b6f0f 100644 --- a/src/borg/archive.py +++ b/src/borg/archive.py @@ -28,6 +28,7 @@ from .constants import * # NOQA from .crypto.low_level import IntegrityError as IntegrityErrorBase from .helpers import BackupError, BackupRaceConditionError, BackupItemExcluded +from .helpers import BackupSymlinkParentError, BackupPathTraversalError from .helpers import BackupOSError, BackupPermissionError, BackupFileNotFoundError, BackupIOError from .helpers import HardLinkManager from .helpers import ChunkIteratorFileWrapper, open_item @@ -523,6 +524,9 @@ def __init__( if not name_is_id: assert len(name) <= 255 self.cwd = os.getcwd() + # cache of parent directory paths verified (during extraction) to be real + # directories and not symlinks, so we do not have to lstat them again and again. + self.safe_dirs = set() assert isinstance(manifest, Manifest) self.manifest = manifest self.key = manifest.repo_objs.key @@ -739,6 +743,44 @@ def calc_stats(self, cache, want_unique=True): stats.osize = self.metadata.size return stats + def _check_safe_parent(self, archived_path): + """Refuse *archived_path* if its parent directory chain is not safe for extraction. + + *archived_path* is a path as stored in the archive, relative to self.cwd. As elsewhere in + borg 2, it is split on "/": archived paths always use "/" as the separator, even on Windows. + borg create never produces a path containing ".." or a path below a symlinked directory, + so such a path can only come from a malicious or corrupted archive and could be used to + access a location outside the extraction directory. + + Raises BackupPathTraversalError if a parent component is "..", or + BackupSymlinkParentError if an existing parent component is a symlink (or other + non-directory). Directories verified to be safe are cached in self.safe_dirs, so each + directory is only lstat'ed once per extraction. + """ + parent_components = [c for c in archived_path.split("/")[:-1] if c not in ("", ".")] + # Reject ".." up front: this keeps the lstat walk below from climbing above self.cwd, + # and makes the early "break" on a not-yet-existing component safe (it can not skip a + # later ".."). + if ".." in parent_components: + raise BackupPathTraversalError(archived_path) + # Every *existing* parent directory component must be a real directory, not a symlink. + current = self.cwd + for component in parent_components: + current = os.path.join(current, component) + if current in self.safe_dirs: + continue + try: + st = os.lstat(current) + except FileNotFoundError: + # parent does not exist yet, it will be created (as a real directory) below an + # already-verified-safe chain. + break + if not stat.S_ISDIR(st.st_mode): + # os.lstat does not follow symlinks, so a symlinked parent shows up here as a + # non-directory. Refuse to follow it out of the extraction directory. + raise BackupSymlinkParentError(archived_path) + self.safe_dirs.add(current) # verified real directory, skip re-stat for later items + @contextmanager def extract_helper(self, item, path, hlm, *, dry_run=False): hardlink_set = False @@ -747,9 +789,18 @@ def extract_helper(self, item, path, hlm, *, dry_run=False): link_target = hlm.retrieve(id=item.hlid) if link_target is not None and has_link: if not dry_run: - # another hard link to same inode (same hlid) was extracted previously, just link to it + # another hard link to same inode (same hlid) was extracted previously, just link to it. + # Security (CVE-2026-62268): follow_symlinks=False makes a hardlinked symlink link the + # symlink itself (a faithful restore) rather than the external file it points to - + # otherwise a crafted archive (symlink -> /etc/shadow, plus a contentless hardlink + # sharing its hlid) could hardlink an arbitrary external file into the extracted tree. + # Guard with supports_follow_symlinks so we do not raise NotImplementedError (which + # backup_io would not catch) on platforms where os.link ignores follow_symlinks. with backup_io("link"): - os.link(link_target, path, follow_symlinks=False) + if os.link in os.supports_follow_symlinks: + os.link(link_target, path, follow_symlinks=False) + else: + os.link(link_target, path) hardlink_set = True yield hardlink_set if not hardlink_set: @@ -836,6 +887,12 @@ def same_item(item, st): dest = self.cwd path = os.path.join(dest, item.path) + # Refuse to extract items whose parent directory path is not safe (a symlinked parent or + # a path containing ".."). Without this check, the "remove existing file" block and the + # open()/mkdir()/symlink() calls below would follow such a parent path and could delete or + # overwrite files outside the extraction directory (e.g. /etc/passwd). + self.safe_dirs.discard(path) # path is about to be (re)created; never trust a stale entry for it + self._check_safe_parent(item.path) # Attempt to remove existing files, ignore errors on failure try: st = os.stat(path, follow_symlinks=False) @@ -858,8 +915,15 @@ def same_item(item, st): def make_parent(path): parent_dir = os.path.dirname(path) + if parent_dir in self.safe_dirs: + # the parent path guard above already verified (this extraction) that parent_dir + # exists and is a real directory, so there is nothing to do and no need to stat it. + return if not os.path.exists(parent_dir): os.makedirs(parent_dir) + # remember parent_dir as a real directory (it either existed - e.g. dest - or we just + # created it below an already-verified-safe chain), so later items can skip the stat. + self.safe_dirs.add(parent_dir) mode = item.mode if stat.S_ISREG(mode): diff --git a/src/borg/helpers/__init__.py b/src/borg/helpers/__init__.py index 5f27f40390..282914e0fa 100644 --- a/src/borg/helpers/__init__.py +++ b/src/borg/helpers/__init__.py @@ -17,6 +17,7 @@ from .errors import BorgWarning, FileChangedWarning, BackupWarning, IncludePatternNeverMatchedWarning from .errors import BackupError, BackupOSError, BackupRaceConditionError, BackupItemExcluded from .errors import BackupPermissionError, BackupIOError, BackupFileNotFoundError +from .errors import BackupSymlinkParentError, BackupPathTraversalError, BackupHardlinkSourceError from .fs import ensure_dir, join_base_dir from .fs import get_security_dir, get_keys_dir, get_base_dir, get_cache_dir, get_config_dir, get_runtime_dir from .fs import dir_is_tagged, dir_is_cachedir, remove_dotdot_prefixes, make_path_safe, scandir_inorder diff --git a/src/borg/helpers/errors.py b/src/borg/helpers/errors.py index 4ea42607ff..5ed00432ef 100644 --- a/src/borg/helpers/errors.py +++ b/src/borg/helpers/errors.py @@ -191,5 +191,31 @@ class BackupFileNotFoundError(BackupOSError): exit_mcode = 107 +class BackupSymlinkParentError(BackupError): + """{}: not extracted, a parent directory is a symlink (malicious or corrupted archive)""" + + # Refusing to extract an item below a symlinked directory (would write outside the + # extraction directory). borg create never produces such items. + exit_mcode = 108 + + +class BackupPathTraversalError(BackupError): + """{}: not extracted, path contains "../" (malicious or corrupted archive)""" + + # Refusing to extract an item whose path contains "..", which could escape the + # extraction directory. borg create never produces such items. + exit_mcode = 109 + + +class BackupHardlinkSourceError(BackupError): + """{}: not extracted, hardlink source path is unsafe (malicious or corrupted archive)""" + + # borg 1.4 raises this when a hardlink's (attacker-controlled) source path contains ".." or + # is below a symlinked directory. borg 2 drives extraction hardlinks by hlid and never uses an + # attacker-controlled archived path as the os.link() source, so nothing raises this here; the + # class (and mcode 110) is kept reserved for cross-branch parity with 1.4-maint. + exit_mcode = 110 + + class BackupItemExcluded(Exception): """Used internally to skip an item from processing when it is excluded.""" diff --git a/src/borg/testsuite/archiver/extract_cmd_test.py b/src/borg/testsuite/archiver/extract_cmd_test.py index a7fca5fe73..05ef2bff91 100644 --- a/src/borg/testsuite/archiver/extract_cmd_test.py +++ b/src/borg/testsuite/archiver/extract_cmd_test.py @@ -9,9 +9,14 @@ from ... import xattr from ... import platform +from ...archive import Archive +from ...cache import Cache from ...chunkers import has_seek_hole from ...constants import * # NOQA -from ...helpers import EXIT_WARNING, BackupPermissionError, bin_to_hex +from ...item import Item +from ...manifest import Manifest +from ...repository import Repository +from ...helpers import EXIT_WARNING, BackupPermissionError, BackupSymlinkParentError, bin_to_hex from ...helpers import flags_noatime, flags_normal from .. import changedir, same_ts_ns, granularity_sleep from .. import are_symlinks_supported, are_hardlinks_supported, is_utime_fully_supported, is_birthtime_fully_supported @@ -46,6 +51,122 @@ def test_symlink_extract(archivers, request): assert os.readlink("input/link1") == "somewhere" +def _create_malicious_archive(archiver, name, items): + # Inject raw Items directly into an archive, bypassing "borg create" (which would never + # produce a child below a symlink or a path with embedded ".."). + repository = Repository(archiver.repository_path, exclusive=True) + with repository: + manifest = Manifest.load(repository, Manifest.NO_OPERATION_CHECK) + with Cache(repository, manifest, archive_name=name) as cache: + archive = Archive(manifest, name, cache=cache, create=True) + for item in items: + archive.items_buffer.add(item) + archive.save(name=name) + + +@pytest.mark.skipif(not are_symlinks_supported(), reason="symlinks not supported") +def test_extract_through_symlinked_parent(archivers, request): + # Security (CVE-2026-62268): an archive with a symlink "evil" -> "", followed by a + # regular file "evil/passwd", must NOT overwrite/delete files in the symlink target directory. + archiver = request.getfixturevalue(archivers) + if archiver.get_kind() != "local": + pytest.skip("malicious archive is crafted via direct (local) repository access") + cmd(archiver, "repo-create", RK_ENCRYPTION) + victim_dir = os.path.join(archiver.tmpdir, "victim") + os.mkdir(victim_dir) + victim_file = os.path.join(victim_dir, "passwd") + with open(victim_file, "w") as fd: + fd.write("original") + attrs = dict(mtime=0, uid=0, gid=0, user="root", group="root") + _create_malicious_archive( + archiver, + "evil", + [ + Item(path="evil", mode=stat.S_IFLNK | 0o777, target=victim_dir, **attrs), + Item(path="evil/passwd", mode=stat.S_IFREG | 0o644, chunks=[], **attrs), + ], + ) + with changedir("output"): + # borg 2 uses modern exit codes, so the warning surfaces as the specific mcode. + output = cmd(archiver, "extract", "evil", exit_code=BackupSymlinkParentError.exit_mcode) + assert "parent directory is a symlink" in output + # the out-of-tree victim must be untouched (neither deleted nor overwritten) + assert os.path.exists(victim_file) + with open(victim_file) as fd: + assert fd.read() == "original" + # the symlink item itself was restored faithfully + assert os.path.islink(os.path.join(archiver.output_path, "evil")) + + +@pytest.mark.skipif( + not (are_symlinks_supported() and are_hardlinks_supported()), reason="symlinks/hardlinks not supported" +) +def test_extract_hardlinked_symlink_does_not_leak_target(archivers, request): + # Security (CVE-2026-62268 Fix 2/2, borg 2 form): a crafted archive with a symlink + # "s" -> "" plus a contentless hardlink "h" sharing its hlid must restore "h" + # as a hardlink to the *symlink* (a faithful restore), NOT as a hardlink to the external + # victim file's inode. The latter (os.link following the symlink) would splice an arbitrary + # external file into the extracted tree - dangerous especially when restoring as root. + archiver = request.getfixturevalue(archivers) + if archiver.get_kind() != "local": + pytest.skip("malicious archive is crafted via direct (local) repository access") + cmd(archiver, "repo-create", RK_ENCRYPTION) + victim_file = os.path.join(archiver.tmpdir, "victim") + with open(victim_file, "w") as fd: + fd.write("secret") + attrs = dict(mtime=0, uid=0, gid=0, user="root", group="root") + hlid = b"H" * 32 # opaque hardlink-group id shared by both items + _create_malicious_archive( + archiver, + "evil", + [ + # master: a symlink pointing at the external victim file + Item(path="s", mode=stat.S_IFLNK | 0o777, target=victim_file, hlid=hlid, **attrs), + # contentless hardlink to the same hlid + Item(path="h", mode=stat.S_IFLNK | 0o777, hlid=hlid, **attrs), + ], + ) + with changedir("output"): + cmd(archiver, "extract", "evil") + s_path = os.path.join(archiver.output_path, "s") + h_path = os.path.join(archiver.output_path, "h") + # "h" is a hardlink to the symlink itself (same inode as "s"), a faithful restore ... + assert os.path.islink(h_path) + assert os.stat(h_path, follow_symlinks=False).st_ino == os.stat(s_path, follow_symlinks=False).st_ino + # ... and NOT a hardlink to the external victim file's inode (that would be the leak). + assert os.stat(h_path, follow_symlinks=False).st_ino != os.stat(victim_file).st_ino + # the out-of-tree victim is untouched + with open(victim_file) as fd: + assert fd.read() == "secret" + + +def test_extract_path_with_embedded_dotdot_rejected_at_item_layer(): + # Security (CVE-2026-62268, embedded-".." vector): unlike borg 1.4, borg 2 already blocks + # this vector one layer earlier - Item.path is sanitized on both encode (assert_sanitized_path) + # and decode (to_sanitized_path), so an item whose path contains ".." can neither be stored in + # an archive nor read back from one. Hence the "../" branch of Archive._check_safe_parent is + # defense-in-depth only and cannot be reached via a crafted archive here; we assert the item + # layer refuses such a path instead. + with pytest.raises(ValueError): + Item(path="a/../../victim/passwd", mode=stat.S_IFREG | 0o644, chunks=[]) + + +def test_extract_deep_tree_ok(archivers, request): + # The parent-path guard (and its safe_dirs cache) must not break normal extraction of a + # deep tree where many files share the same parent directories. + archiver = request.getfixturevalue(archivers) + cmd(archiver, "repo-create", RK_ENCRYPTION) + create_regular_file(archiver.input_path, "d1/d2/d3/file1", size=10) + create_regular_file(archiver.input_path, "d1/d2/d3/file2", size=10) + create_regular_file(archiver.input_path, "d1/d2/other", size=10) + cmd(archiver, "create", "test", "input") + with changedir("output"): + cmd(archiver, "extract", "test") + assert os.path.exists("input/d1/d2/d3/file1") + assert os.path.exists("input/d1/d2/d3/file2") + assert os.path.exists("input/d1/d2/other") + + def test_extract_stats(archivers, request): archiver = request.getfixturevalue(archivers) create_regular_file(archiver.input_path, "file1", size=1024)