diff --git a/NEWS.rst b/NEWS.rst index 1dcfbd480a..8a11614d0c 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -9,6 +9,62 @@ .. towncrier release notes start +20.3.4.post3 (2026-10-21) +========================= + +Security Fixes +-------------- + +- **CVE-2026-6357**: Run the pip self-version check before the command body + and only report its result afterwards, so ``pip install`` can no longer be + made to import and execute a module from a just-installed wheel. The check + computes supported tags, and the vendored ``packaging`` probes + ``import _manylinux`` (the PEP 513/600 override hook) every time it does. + (`GHSA-jp4c-xjxw-mgf9 `_) +- **CVE-2025-8869** (hardening): Port upstream's follow-up fix (pypa/pip#14127) + to the tar extraction path, which is the only one on Python 2.7 (there is no + ``tarfile.data_filter``). A member is now rejected unless its resolved + (``realpath``) destination stays inside the extract directory, and a symlink + is rejected when its resolved target leaves it. Previously a symlink to + ``.`` or ``..`` (both archive members) let a later member be written outside + the extract directory. 20.3.4+security.1 and 20.3.4.post2 are affected. + (`GHSA-4xh5-x5gv-qwph `_) +- **CVE-2023-5752** (regression fix): Pass a Mercurial revision as + ``--rev=`` instead of ``-r=`` (pypa/pip#12373). Mercurial parses + ``-r=`` as the revision ``=`` and fails, so every + ``hg+...@`` install has been broken since 20.3.4+security.1 (also in + 20.3.4.post2). The revision is still a single token, so the CVE protection + is unchanged. + (`GHSA-mq26-g339-26xf `_) + +Bug Fixes +--------- + +- ``is_within_directory`` and the CVE-2026-8643 entry point check accept a + directory that is a filesystem root (``/`` or ``C:\``); previously every + target, and every entry point, was rejected there. +- The pre-command version-check lookup now runs inside pip's error handling, + so Ctrl-C during it reports "Operation cancelled by user", and a malformed + requirement such as ``foo @ http://[bad/x`` gets pip's normal error report + instead of a raw traceback. +- Symlinks in an sdist are matched against normalized member names, including + implied parent directories, so ``./``-prefixed archives (``tar -C dir .``) + and links to a directory without its own entry unpack again. Containment is + still enforced by the ``realpath`` checks above. +- A download whose file name is not a single safe path component (e.g. + ``Content-Disposition: filename=".."``) is reported as a pip ``ERROR:`` + line instead of a traceback. It was already refused. + +.. note:: + + This corrects the 20.3.4+security.1 notes, which assessed CVE-2026-6357 + as **not applicable** to 20.3.4. That was wrong: a failed import is never + cached in ``sys.modules``, so the ``_manylinux`` probe is retried by the + post-install check and picks up a top-level ``_manylinux.py`` that a wheel + has just installed. 20.3.4+security.1 and 20.3.4.post2 are affected; + ``--disable-pip-version-check`` avoids it on those releases. + + 20.3.4.post2 (2026-10-21) ========================= @@ -33,7 +89,7 @@ Security Fixes (`GHSA-43fp-rhv2-5gv8 `_, `GHSA-xqr8-7jwr-rhp7 `_) -.. note +.. note:: CVE-2024-39689 / GHSA-248v-346w-9cwc (certifi GLOBALTRUST roots) was assessed and found **not applicable**: those roots were added in certifi @@ -67,7 +123,7 @@ Security Fixes (instead of a character-prefix comparison) so an extracted file cannot land in a sibling directory whose name is a prefix of the install path. (`GHSA-6vgw-5pg2-w6jp `_) -.. note +.. note:: CVE-2026-6357 / GHSA-jp4c-xjxw-mgf9 was assessed and found **not applicable** to this release: pip 20.3.4 imports the self-version-check and all of its @@ -75,6 +131,8 @@ Security Fixes vendored (``pip._vendor.*``) modules, so a newly-installed wheel has no well-known module name to shadow. + (Corrected in 20.3.4.post3: this release *is* affected, see above.) + 20.3.4 (2021-01-23) =================== diff --git a/src/pip/__init__.py b/src/pip/__init__.py index 2a094febd8..15135db80f 100644 --- a/src/pip/__init__.py +++ b/src/pip/__init__.py @@ -4,7 +4,7 @@ from typing import List, Optional -__version__ = "20.3.4.post2" +__version__ = "20.3.4.post3" def main(args=None): diff --git a/src/pip/_internal/cli/base_command.py b/src/pip/_internal/cli/base_command.py index 41e7dcf101..1e322b4c18 100644 --- a/src/pip/_internal/cli/base_command.py +++ b/src/pip/_internal/cli/base_command.py @@ -100,6 +100,25 @@ def handle_pip_version_check(self, options): # are present. assert not hasattr(options, 'no_index') + def _pip_version_check_fetch(self, options, args): + # type: (Values, List[str]) -> Any + """Gather what the pip version check needs, before the command runs. + + The result is passed to _pip_version_check_emit() once the command + body has finished. Doing the lookups up front means nothing is + imported after packages have been installed (CVE-2026-6357). + """ + return None + + def _pip_version_check_emit(self, options, fetched): + # type: (Values, Any) -> None + """Report the pip version check after the command has run. + + By default this calls handle_pip_version_check(), which is a no-op + for commands without the index_group options. + """ + self.handle_pip_version_check(options) + def run(self, options, args): # type: (Values, List[Any]) -> int raise NotImplementedError @@ -219,7 +238,13 @@ def _main(self, args): "This will become an error in pip 21.0." ) + # The version check does its lookups before the command body runs and + # only reports afterwards, so nothing is imported once packages have + # been installed (CVE-2026-6357). The fetch runs inside the try so + # its errors (and Ctrl-C) get pip's normal error handling. + version_check = None try: + version_check = self._pip_version_check_fetch(options, args) status = self.run(options, args) assert isinstance(status, int) return status @@ -257,4 +282,4 @@ def _main(self, args): return UNKNOWN_ERROR finally: - self.handle_pip_version_check(options) + self._pip_version_check_emit(options, version_check) diff --git a/src/pip/_internal/cli/req_command.py b/src/pip/_internal/cli/req_command.py index 008066ab1c..cd93c1cdce 100644 --- a/src/pip/_internal/cli/req_command.py +++ b/src/pip/_internal/cli/req_command.py @@ -27,7 +27,11 @@ install_req_from_req_string, ) from pip._internal.req.req_file import parse_requirements -from pip._internal.self_outdated_check import pip_self_version_check +from pip._internal.self_outdated_check import ( + _pip_self_version_check_emit, + _pip_self_version_check_fetch, + pip_self_version_check, +) from pip._internal.utils.temp_dir import tempdir_kinds from pip._internal.utils.typing import MYPY_CHECK_RUNNING @@ -152,6 +156,43 @@ def handle_pip_version_check(self, options): with session: pip_self_version_check(session, options) + def _pip_version_check_fetch(self, options, args): + # type: (Values, List[str]) -> Any + """Query the index for the latest pip before the command body runs. + + handle_pip_version_check() does the same check in one step, but run + after `pip install` it would compute supported tags, which probes + ``import _manylinux``, and could import a module that a just-installed + wheel provided (CVE-2026-6357). So _main() fetches here and only + reports in _pip_version_check_emit(). + """ + # Make sure the index_group options are present. + assert hasattr(options, 'no_index') + + if options.disable_pip_version_check or options.no_index: + return None + + try: + session = self._build_session( + options, + retries=0, + timeout=min(5, options.timeout) + ) + with session: + return _pip_self_version_check_fetch(session, options) + except Exception: + logger.debug( + "There was an error checking the latest version of pip", + exc_info=True, + ) + return None + + def _pip_version_check_emit(self, options, fetched): + # type: (Values, Any) -> None + # Only report what was fetched before the command ran. Do not fall + # back to handle_pip_version_check(), which would query again now. + _pip_self_version_check_emit(fetched) + KEEPABLE_TEMPDIR_TYPES = [ tempdir_kinds.BUILD_ENV, diff --git a/src/pip/_internal/commands/install.py b/src/pip/_internal/commands/install.py index 0f6c384e5c..4f112c511a 100644 --- a/src/pip/_internal/commands/install.py +++ b/src/pip/_internal/commands/install.py @@ -9,6 +9,7 @@ from optparse import SUPPRESS_HELP from pip._vendor import pkg_resources +from pip._vendor.packaging.requirements import InvalidRequirement, Requirement from pip._vendor.packaging.utils import canonicalize_name from pip._internal.cache import WheelCache @@ -37,7 +38,7 @@ if MYPY_CHECK_RUNNING: from optparse import Values - from typing import Iterable, List, Optional + from typing import Any, Iterable, List, Optional from pip._internal.models.format_control import FormatControl from pip._internal.operations.check import ConflictDetails @@ -61,6 +62,17 @@ def check_binary_allowed(req): return check_binary_allowed +def _arg_refers_to_pip(arg): + # type: (str) -> bool + try: + req = Requirement(arg) + except (InvalidRequirement, ValueError): + # A malformed requirement (e.g. a URL that urlparse rejects with + # ValueError) is not pip; leave the error to the normal parsing path. + return False + return canonicalize_name(req.name) == "pip" + + class InstallCommand(RequirementCommand): """ Install packages from: @@ -225,6 +237,17 @@ def add_options(self): self.parser.insert_option_group(0, index_opts) self.parser.insert_option_group(0, self.cmd_opts) + def _pip_version_check_fetch(self, options, args): + # type: (Values, List[str]) -> Any + # Skip the self-version check when pip itself is a requirement. The + # running pip may be replaced mid-command, and the upgrade prompt + # is redundant. + if any(_arg_refers_to_pip(arg) for arg in args): + return None + return super(InstallCommand, self)._pip_version_check_fetch( + options, args + ) + @with_cleanup def run(self, options, args): # type: (Values, List[str]) -> int diff --git a/src/pip/_internal/exceptions.py b/src/pip/_internal/exceptions.py index 8d7f7fa39b..47f6769e7a 100644 --- a/src/pip/_internal/exceptions.py +++ b/src/pip/_internal/exceptions.py @@ -36,6 +36,16 @@ class InstallationError(PipError): """General exception during installation""" +class UnsafeFilenameError(InstallationError, ValueError): + """A file name derived from a URL or header is not a single safe path + component. + + It is also a ValueError, so callers written against the plain + ValueError keep working, while pip's command loop reports it as an + ordinary ``ERROR:`` line instead of a traceback. + """ + + class UninstallationError(PipError): """General exception during uninstallation""" diff --git a/src/pip/_internal/models/link.py b/src/pip/_internal/models/link.py index 129c4b2120..e646ece135 100644 --- a/src/pip/_internal/models/link.py +++ b/src/pip/_internal/models/link.py @@ -4,6 +4,7 @@ from pip._vendor.six.moves.urllib import parse as urllib_parse +from pip._internal.exceptions import UnsafeFilenameError from pip._internal.utils.filetypes import WHEEL_EXTENSION from pip._internal.utils.misc import ( redact_auth_from_url, @@ -44,7 +45,7 @@ def as_path_component(name): """ component = _to_path_component(name) if not component: - raise ValueError( + raise UnsafeFilenameError( "Unexpected file name derived from URL: {!r}".format(name) ) @@ -61,7 +62,7 @@ def join_within_directory(directory, component): has no runtime type to enforce that, so it is checked here instead. """ if not component or component != _to_path_component(component): - raise ValueError( + raise UnsafeFilenameError( "Not a single path component: {!r}".format(component) ) diff --git a/src/pip/_internal/operations/install/wheel.py b/src/pip/_internal/operations/install/wheel.py index 59ca06f8d5..0d97ef51fe 100644 --- a/src/pip/_internal/operations/install/wheel.py +++ b/src/pip/_internal/operations/install/wheel.py @@ -478,7 +478,9 @@ def _script_within_dir(name, scripts_dir): """ root = os.path.normpath(scripts_dir) dest = os.path.normpath(os.path.join(scripts_dir, name)) - return dest.startswith(root + os.sep) + # The script must be a file *inside* the directory, never the directory + # itself. + return dest != root and is_within_directory(root, dest) def _raise_for_invalid_entrypoint(specification, scripts_dir): diff --git a/src/pip/_internal/self_outdated_check.py b/src/pip/_internal/self_outdated_check.py index c2d166b184..cb10de6f1c 100644 --- a/src/pip/_internal/self_outdated_check.py +++ b/src/pip/_internal/self_outdated_check.py @@ -20,7 +20,7 @@ if MYPY_CHECK_RUNNING: import optparse - from typing import Any, Dict, Text, Union + from typing import Any, Dict, Optional, Text, Tuple, Union from pip._internal.network.session import PipSession @@ -118,10 +118,32 @@ def pip_self_version_check(session, options): Limit the frequency of checks to once per week. State is stored either in the active virtualenv or in the user's USER_CACHE_DIR keyed off the prefix of the pip script path. + + This fetches and reports in one go. Commands run the two halves + separately (_pip_self_version_check_fetch before the command body, + _pip_self_version_check_emit after it) so that nothing is looked up or + imported once packages have been installed (CVE-2026-6357). + """ + _pip_self_version_check_emit( + _pip_self_version_check_fetch(session, options) + ) + + +def _pip_self_version_check_fetch(session, options): + # type: (PipSession, optparse.Values) -> Optional[Tuple[str, str]] + """Do all the work of the pip self-version check, except reporting. + + Returns ``(installed_version, pypi_version)`` when an upgrade prompt + should be shown, otherwise None. This queries the index, computes the + supported tags and inspects the installed pip distribution, so it must + run before a command can install anything: the vendored packaging probes + ``import _manylinux`` while computing tags, and a failed import is not + cached, so after an install it would import whatever module of that name + a just-installed wheel provided (CVE-2026-6357). """ installed_version = get_installed_version("pip") if not installed_version: - return + return None pip_version = packaging_version.parse(installed_version) pypi_version = None @@ -161,7 +183,7 @@ def pip_self_version_check(session, options): ) best_candidate = finder.find_best_candidate("pip").best_candidate if best_candidate is None: - return + return None pypi_version = str(best_candidate.version) # save that we've performed a check @@ -177,21 +199,36 @@ def pip_self_version_check(session, options): # Determine if our pypi_version is older if not local_version_is_older: - return + return None - # We cannot tell how the current pip is available in the current - # command context, so be pragmatic here and suggest the command - # that's always available. This does not accommodate spaces in - # `sys.executable`. - pip_cmd = "{} -m pip".format(sys.executable) - logger.warning( - "You are using pip version %s; however, version %s is " - "available.\nYou should consider upgrading via the " - "'%s install --upgrade pip' command.", - pip_version, pypi_version, pip_cmd - ) + return str(pip_version), str(pypi_version) except Exception: logger.debug( "There was an error checking the latest version of pip", exc_info=True, ) + return None + + +def _pip_self_version_check_emit(upgrade_prompt): + # type: (Optional[Tuple[str, str]]) -> None + """Report the result of _pip_self_version_check_fetch, if any. + + Only logs: it must not query the index or import anything, because it + runs after the command body (CVE-2026-6357). + """ + if upgrade_prompt is None: + return + + pip_version, pypi_version = upgrade_prompt + # We cannot tell how the current pip is available in the current + # command context, so be pragmatic here and suggest the command + # that's always available. This does not accommodate spaces in + # `sys.executable`. + pip_cmd = "{} -m pip".format(sys.executable) + logger.warning( + "You are using pip version %s; however, version %s is " + "available.\nYou should consider upgrading via the " + "'%s install --upgrade pip' command.", + pip_version, pypi_version, pip_cmd + ) diff --git a/src/pip/_internal/utils/unpacking.py b/src/pip/_internal/utils/unpacking.py index 2a926e648c..740390c109 100644 --- a/src/pip/_internal/utils/unpacking.py +++ b/src/pip/_internal/utils/unpacking.py @@ -5,6 +5,7 @@ import logging import os +import posixpath import shutil import stat import tarfile @@ -21,7 +22,7 @@ from pip._internal.utils.typing import MYPY_CHECK_RUNNING if MYPY_CHECK_RUNNING: - from typing import Iterable, List, Optional, Text, Union + from typing import Iterable, List, Optional, Set, Text, Union from zipfile import ZipInfo @@ -84,41 +85,72 @@ def has_leading_dir(paths): return True -def is_within_directory(directory, target): - # type: ((Union[str, Text]), (Union[str, Text])) -> bool +def is_within_directory(directory, target, resolve_symlinks=False): + # type: ((Union[str, Text]), (Union[str, Text]), bool) -> bool """ Return true if the absolute path of target is within the directory + (including when target is equal to the directory). + + When ``resolve_symlinks`` is true, resolve symlinks before comparing so + traversal through a symlink (e.g. "link/../file") is also caught. """ - abs_directory = os.path.abspath(directory) - abs_target = os.path.abspath(target) + if resolve_symlinks: + abs_directory = os.path.realpath(directory) + abs_target = os.path.realpath(target) + else: + abs_directory = os.path.abspath(directory) + abs_target = os.path.abspath(target) # NOTE: os.path.commonprefix compares character-by-character, so it treats # "/a/parent" and "/a/parentfoo" as sharing the prefix "/a/parent" # and would report the latter as "within" the former (CVE-2026-1703). # os.path.commonpath compares on path components but is unavailable on - # Python 2.7, so enforce a component boundary explicitly instead. - return abs_target == abs_directory or abs_target.startswith( - abs_directory + os.sep - ) - - -def is_symlink_target_in_tar(tar, tarinfo): - # type: (tarfile.TarFile, tarfile.TarInfo) -> bool + # Python 2.7 (and drops a POSIX "//" prefix), so enforce a component + # boundary explicitly instead. + if abs_target == abs_directory: + return True + # A filesystem root ("/" or "C:\\") already ends with a separator; + # appending another would reject everything. + if abs_directory.endswith(os.sep): + prefix = abs_directory + else: + prefix = abs_directory + os.sep + return abs_target.startswith(prefix) + + +def _tar_member_paths(tar): + # type: (tarfile.TarFile) -> Set[str] + """Normalized member paths plus every implied parent directory.""" + paths = set() # type: Set[str] + for member in tar.getmembers(): + name = posixpath.normpath(member.name.replace("\\", "/")) + while name not in ("", ".", "/"): + paths.add(name) + parent = posixpath.dirname(name) + if parent == name: + # posixpath.dirname("//") == "//": stop at any fixed point. + break + name = parent + return paths + + +def is_symlink_target_in_tar(tar, tarinfo, member_paths=None): + # type: (tarfile.TarFile, tarfile.TarInfo, Optional[Set[str]]) -> bool """ Return true if the file the symlink member points to is itself a member of the same tar archive, i.e. the symlink does not escape the archive to reference an arbitrary path on the host filesystem (CVE-2025-8869). + + Member names are compared after normalization, and directories implied + by a member's path count as members, so "./"-prefixed archives and links + to a directory without its own entry are accepted. Containment itself is + enforced by the realpath checks in untar_file(). """ linkname = os.path.join(os.path.dirname(tarinfo.name), tarinfo.linkname) - - linkname = os.path.normpath(linkname) - linkname = linkname.replace("\\", "/") - - try: - tar.getmember(linkname) - return True - except KeyError: - return False + linkname = posixpath.normpath(linkname.replace("\\", "/")) + if member_paths is None: + member_paths = _tar_member_paths(tar) + return linkname in member_paths def set_extracted_file_to_default_mode_plus_executable(path): @@ -214,13 +246,21 @@ def untar_file(filename, location): leading = has_leading_dir([ member.name for member in tar.getmembers() ]) + # Computed once: is_symlink_target_in_tar() would otherwise rebuild + # it for every symlink member. + member_paths = _tar_member_paths(tar) for member in tar.getmembers(): fn = member.name if leading: # https://github.com/python/mypy/issues/1174 fn = split_leading_dir(fn)[1] # type: ignore path = os.path.join(location, fn) - if not is_within_directory(location, path): + # The plain check rejects textual ".." escapes; resolving symlinks + # also catches a later member redirected outside by an earlier + # member's symlink (e.g. "link/../file"). + if not is_within_directory(location, path) or ( + not is_within_directory(location, path, resolve_symlinks=True) + ): message = ( 'The tar file ({}) has a file ({}) trying to install ' 'outside target directory ({})' @@ -231,7 +271,22 @@ def untar_file(filename, location): if member.isdir(): ensure_dir(path) elif member.issym(): - if not is_symlink_target_in_tar(tar, member): + # Reject symlinks resolving outside the destination, so a + # later member cannot be written through them. + link_target = os.path.join( + os.path.dirname(path), member.linkname + ) + if not is_within_directory( + location, link_target, resolve_symlinks=True + ): + message = ( + 'The tar file ({}) has a link ({}) trying to point ' + 'outside target directory ({})' + ) + raise InstallationError( + message.format(filename, member.name, member.linkname) + ) + if not is_symlink_target_in_tar(tar, member, member_paths): message = ( 'The tar file ({}) has a link ({}) trying to point ' 'outside target directory ({})' diff --git a/src/pip/_internal/vcs/mercurial.py b/src/pip/_internal/vcs/mercurial.py index 1fc7f0d204..8b8e67e885 100644 --- a/src/pip/_internal/vcs/mercurial.py +++ b/src/pip/_internal/vcs/mercurial.py @@ -38,10 +38,12 @@ class Mercurial(VersionControl): @staticmethod def get_base_rev_args(rev): - # CVE-2023-5752: pass the revision as a single ``-r=`` token so a - # malicious revision (e.g. ``--config=...``) cannot be interpreted by - # hg as a separate command-line option. - return ["-r={}".format(rev)] + # CVE-2023-5752: pass the revision as a single ``--rev=`` token + # so a malicious revision (e.g. ``--config=...``) cannot be + # interpreted by hg as a separate command-line option. The short form + # ``-r=`` must not be used: hg reads it as the revision ``=`` + # and fails ("parse error ... not a prefix: =") (pypa/pip#12373). + return ["--rev={}".format(rev)] def export(self, location, url): # type: (str, HiddenText) -> None diff --git a/tests/unit/test_base_command.py b/tests/unit/test_base_command.py index 8ba3d9e252..4ae13d5cf6 100644 --- a/tests/unit/test_base_command.py +++ b/tests/unit/test_base_command.py @@ -96,6 +96,64 @@ def test_handle_pip_version_check_called(mock_handle_version_check): mock_handle_version_check.assert_called_once() +def test_version_check_fetch_before_run_emit_after(monkeypatch): + """ + CVE-2026-6357: _main() fetches for the version check before the command + body and reports after it. + """ + events = [] + + def fetch(self, options, args): + events.append('fetch') + return 'fetched' + + def emit(self, options, fetched): + events.append(('emit', fetched)) + + monkeypatch.setattr(Command, '_pip_version_check_fetch', fetch) + monkeypatch.setattr(Command, '_pip_version_check_emit', emit) + + def run_func(): + events.append('run') + return SUCCESS + + cmd = FakeCommand(run_func=run_func) + assert cmd.main([]) == SUCCESS + assert events == ['fetch', 'run', ('emit', 'fetched')] + + +@pytest.mark.parametrize('exc, message', [ + (KeyboardInterrupt, 'Operation cancelled by user'), + (ValueError, 'Exception:'), +]) +def test_version_check_fetch_errors_are_handled( + monkeypatch, capsys, exc, message +): + """ + The pre-command version-check fetch runs inside _main()'s error + handling, so Ctrl-C or an unexpected error during it is reported by pip + instead of escaping as a raw traceback. + """ + emitted = [] + + def fetch(self, options, args): + raise exc() + + def emit(self, options, fetched): + emitted.append(fetched) + + monkeypatch.setattr(Command, '_pip_version_check_fetch', fetch) + monkeypatch.setattr(Command, '_pip_version_check_emit', emit) + + cmd = FakeCommand(run_func=lambda: pytest.fail('run() must not be called')) + status = cmd.main([]) + assert status != SUCCESS + stderr = capsys.readouterr().err + assert message in stderr + # The emit step still runs (with nothing fetched). + assert emitted == [None] + + def test_log_command_success(fixed_time, tmpdir): """Test the --log option logs when command succeeds.""" cmd = FakeCommand() diff --git a/tests/unit/test_commands.py b/tests/unit/test_commands.py index 7fae427c69..61bde71fd7 100644 --- a/tests/unit/test_commands.py +++ b/tests/unit/test_commands.py @@ -6,7 +6,9 @@ RequirementCommand, SessionCommandMixin, ) +from pip._internal.cli.status_codes import SUCCESS from pip._internal.commands import commands_dict, create_command +from pip._internal.commands.install import _arg_refers_to_pip # These are the expected names of the commands whose classes inherit from # IndexGroupCommand. @@ -111,3 +113,111 @@ def is_requirement_command(command): return isinstance(command, RequirementCommand) check_commands(is_requirement_command, ['download', 'install', 'wheel']) + + +@pytest.mark.parametrize('command_name', EXPECTED_INDEX_GROUP_COMMANDS) +def test_index_group_version_check_runs_before_command_body( + monkeypatch, command_name, +): + """ + CVE-2026-6357: the self-version check must do its lookups before the + command body (which may install packages) and only report afterwards. + Run after `pip install`, the lookup computes supported tags, which + probes ``import _manylinux`` and could execute a just-installed module. + """ + # The isolate fixture disables the check through the environment. + monkeypatch.delenv('PIP_DISABLE_PIP_VERSION_CHECK', raising=False) + events = [] + fetched = object() + + def fake_fetch(session, options): + events.append('fetch') + return fetched + + def fake_emit(prompt): + events.append(('emit', prompt is fetched)) + + def fake_full_check(session, options): + events.append('full-check') + + monkeypatch.setattr( + 'pip._internal.cli.req_command._pip_self_version_check_fetch', + fake_fetch, + ) + monkeypatch.setattr( + 'pip._internal.cli.req_command._pip_self_version_check_emit', + fake_emit, + ) + monkeypatch.setattr( + 'pip._internal.cli.req_command.pip_self_version_check', + fake_full_check, + ) + + command = create_command(command_name) + + def fake_run(options, args): + events.append('run') + return SUCCESS + + monkeypatch.setattr(command, 'run', fake_run) + + assert command.main([]) == SUCCESS + assert events == ['fetch', 'run', ('emit', True)] + + +@pytest.mark.parametrize('command_name', EXPECTED_INDEX_GROUP_COMMANDS) +@pytest.mark.parametrize('extra_args', [ + ['--disable-pip-version-check'], + ['--no-index'], +]) +def test_index_group_version_check_disabled( + monkeypatch, command_name, extra_args, +): + events = [] + monkeypatch.setattr( + 'pip._internal.cli.req_command._pip_self_version_check_fetch', + lambda session, options: events.append('fetch'), + ) + monkeypatch.setattr( + 'pip._internal.cli.req_command._pip_self_version_check_emit', + lambda prompt: events.append(('emit', prompt)), + ) + command = create_command(command_name) + monkeypatch.setattr(command, 'run', lambda options, args: SUCCESS) + + assert command.main(extra_args) == SUCCESS + assert events == [('emit', None)] + + +@pytest.mark.parametrize('arg, expected', [ + ('pip', True), + ('PIP==20.3.4', True), + ('pip>=20', True), + ('pip[foo]', True), + ('pipx', False), + ('six', False), + ('./pip-20.3.4-py2.py3-none-any.whl', False), + ('-r', False), + # urlparse raises ValueError for this URL; it is simply not pip. + ('foo @ http://[bad/x', False), +]) +def test_arg_refers_to_pip(arg, expected): + assert _arg_refers_to_pip(arg) is expected + + +def test_install_skips_version_check_when_installing_pip(monkeypatch): + calls = [] + monkeypatch.setattr( + 'pip._internal.cli.req_command._pip_self_version_check_fetch', + lambda session, options: calls.append('fetch'), + ) + command = create_command('install') + options = command.parser.get_default_values() + options.disable_pip_version_check = False + options.no_index = False + + assert command._pip_version_check_fetch(options, ['pip']) is None + assert calls == [] + + command._pip_version_check_fetch(options, ['six']) + assert calls == ['fetch'] diff --git a/tests/unit/test_link.py b/tests/unit/test_link.py index 3788f5a8c2..c363180bbb 100644 --- a/tests/unit/test_link.py +++ b/tests/unit/test_link.py @@ -3,6 +3,7 @@ import pytest +from pip._internal.exceptions import InstallationError, UnsafeFilenameError from pip._internal.models.link import ( Link, as_path_component, @@ -240,3 +241,16 @@ def test_join_within_directory_rejects_non_component(component): # its argument itself instead of trusting the caller. with pytest.raises(ValueError): join_within_directory('downloads', component) + + +@pytest.mark.parametrize('call', [ + lambda: as_path_component('..'), + lambda: join_within_directory('downloads', '..'), +]) +def test_unsafe_filename_is_a_pip_installation_error(call): + # Reported by pip's command loop as an "ERROR:" line, not a traceback, + # while staying a ValueError for existing callers. + with pytest.raises(UnsafeFilenameError) as e: + call() + assert isinstance(e.value, InstallationError) + assert isinstance(e.value, ValueError) diff --git a/tests/unit/test_self_check_outdated.py b/tests/unit/test_self_check_outdated.py index c5e60d92fc..6a92f9e0a9 100644 --- a/tests/unit/test_self_check_outdated.py +++ b/tests/unit/test_self_check_outdated.py @@ -11,6 +11,8 @@ from pip._internal.models.candidate import InstallationCandidate from pip._internal.self_outdated_check import ( SelfCheckState, + _pip_self_version_check_emit, + _pip_self_version_check_fetch, logger, pip_self_version_check, ) @@ -139,6 +141,76 @@ def test_pip_self_version_check(monkeypatch, stored_time, installed_ver, assert len(logger.warning.calls) == 0 +def _patch_upgrade_available(monkeypatch): + monkeypatch.setattr(self_outdated_check, 'get_installed_version', + lambda name: '1.0') + monkeypatch.setattr(self_outdated_check, 'PackageFinder', + MockPackageFinder) + monkeypatch.setattr(self_outdated_check, 'get_distribution', + lambda name: MockDistribution('pip')) + monkeypatch.setattr(logger, 'warning', + pretend.call_recorder(lambda *a, **kw: None)) + fake_state = pretend.stub( + state={}, + save=pretend.call_recorder(lambda v, t: None), + ) + monkeypatch.setattr( + self_outdated_check, 'SelfCheckState', lambda **kw: fake_state + ) + + +def test_self_version_check_fetch_does_not_log(monkeypatch): + # CVE-2026-6357: the fetch half runs before the command body, the emit + # half only reports afterwards. + _patch_upgrade_available(monkeypatch) + + prompt = _pip_self_version_check_fetch(None, _options()) + + assert prompt == ('1.0', '6.9.0') + assert logger.warning.calls == [] + + +def test_self_version_check_emit_does_no_lookups(monkeypatch): + # The emit half runs after `pip install`; it must not query the index, + # compute tags or inspect installed distributions (CVE-2026-6357). + def fail(*args, **kwargs): + raise AssertionError("lookup after the command body") + + monkeypatch.setattr(self_outdated_check, 'get_installed_version', fail) + monkeypatch.setattr(self_outdated_check, 'get_distribution', fail) + monkeypatch.setattr(self_outdated_check, 'PackageFinder', + pretend.stub(create=fail)) + monkeypatch.setattr(self_outdated_check, 'SelfCheckState', fail) + monkeypatch.setattr(logger, 'warning', + pretend.call_recorder(lambda *a, **kw: None)) + + _pip_self_version_check_emit(('1.0', '6.9.0')) + + assert len(logger.warning.calls) == 1 + assert logger.warning.calls[0].args[1:3] == ('1.0', '6.9.0') + + +def test_self_version_check_emit_none_is_silent(monkeypatch): + monkeypatch.setattr(logger, 'warning', + pretend.call_recorder(lambda *a, **kw: None)) + _pip_self_version_check_emit(None) + assert logger.warning.calls == [] + + +def test_self_version_check_fetch_swallows_errors(monkeypatch): + _patch_upgrade_available(monkeypatch) + + def broken(**kwargs): + raise RuntimeError("boom") + + monkeypatch.setattr(self_outdated_check, 'SelfCheckState', broken) + monkeypatch.setattr(logger, 'debug', + pretend.call_recorder(lambda *a, **kw: None)) + + assert _pip_self_version_check_fetch(None, _options()) is None + assert len(logger.debug.calls) == 1 + + statefile_name_case_1 = ( "fcd2d5175dd33d5df759ee7b045264230205ef837bf9f582f7c3ada7" ) diff --git a/tests/unit/test_utils_unpacking.py b/tests/unit/test_utils_unpacking.py index 5162cecf94..76d2e5bc73 100644 --- a/tests/unit/test_utils_unpacking.py +++ b/tests/unit/test_utils_unpacking.py @@ -12,6 +12,7 @@ from pip._internal.exceptions import InstallationError from pip._internal.utils.unpacking import ( + _tar_member_paths, is_within_directory, unpack_file, untar_file, @@ -209,6 +210,122 @@ def test_unpack_tar_symlink_to_member_success(self): assert os.path.islink(link_path) assert os.readlink(link_path) == 'target.txt' + def _make_tar(self, name, entries): + path = os.path.join(self.tempdir, name) + with tarfile.open(path, 'w') as tar: + for member_name, kind, arg in entries: + info = tarfile.TarInfo(member_name) + if kind == 'dir': + info.type = tarfile.DIRTYPE + tar.addfile(info) + elif kind == 'sym': + info.type = tarfile.SYMTYPE + info.linkname = arg + tar.addfile(info) + else: + info.size = len(arg) + tar.addfile(info, io.BytesIO(arg)) + return path + + def test_unpack_tar_dot_prefixed_symlink_success(self): + """A "./"-prefixed archive (``tar -C dir .``) with an in-archive + symlink is accepted: member names are compared normalized.""" + test_tar = self._make_tar('dot_prefixed.tar', [ + ('./', 'dir', None), + ('./pkg', 'dir', None), + ('./pkg/target.txt', 'file', b'data\n'), + ('./pkg/link', 'sym', 'target.txt'), + ]) + out = os.path.join(self.tempdir, 'out') + untar_file(test_tar, out) + assert os.readlink(os.path.join(out, 'pkg', 'link')) == 'target.txt' + + def test_unpack_tar_symlink_to_implied_dir_success(self): + """A symlink to a directory that has no entry of its own (only files + beneath it) is accepted: implied parent directories count.""" + test_tar = self._make_tar('implied_dir.tar', [ + ('pkg/sub/f.txt', 'file', b'data\n'), + ('pkg/link', 'sym', 'sub'), + ]) + out = os.path.join(self.tempdir, 'out') + untar_file(test_tar, out) + assert os.readlink(os.path.join(out, 'link')) == 'sub' + + def test_unpack_tar_symlink_to_non_member_failure(self): + """A symlink to a name that is not in the archive is still refused.""" + test_tar = self._make_tar('dangling.tar', [ + ('pkg/a.txt', 'file', b'data\n'), + ('pkg/link', 'sym', 'missing.txt'), + ]) + with pytest.raises(InstallationError) as e: + untar_file(test_tar, os.path.join(self.tempdir, 'out')) + assert 'trying to point outside target directory' in str(e.value) + + def test_unpack_tar_symlink_then_member(self): + """Reject a symlink to outside before a member is written through it. + + CVE-2025-8869 follow-up (pypa/pip#14127). Python 2.7 has no + ``tarfile.data_filter``, so this check is the only protection. + """ + tar_filepath = os.path.join(self.tempdir, 'symlink_then_member.tar') + extract_path = os.path.join(self.tempdir, 'extract_path') + outside_path = os.path.join(self.tempdir, 'outside.txt') + + with tarfile.open(tar_filepath, 'w') as tar: + info = tarfile.TarInfo('outside_link') + info.type = tarfile.SYMTYPE + info.linkname = '..' + tar.addfile(info) + + data = b'data\n' + info = tarfile.TarInfo('outside_link/outside.txt') + info.size = len(data) + tar.addfile(info, fileobj=io.BytesIO(data)) + + info = tarfile.TarInfo('..') + info.type = tarfile.DIRTYPE + tar.addfile(info) + + with pytest.raises(InstallationError): + untar_file(tar_filepath, extract_path) + + assert not os.path.exists(outside_path) + assert not os.path.exists(os.path.join(extract_path, 'outside_link')) + + def test_unpack_tar_nested_symlink_traversal(self): + """Reject a member that escapes through a chain of in-bounds symlinks. + + CVE-2025-8869 follow-up (pypa/pip#14127). + """ + tar_filepath = os.path.join(self.tempdir, 'nested_traversal.tar') + extract_path = os.path.join(self.tempdir, 'extract_path') + outside_path = os.path.join(self.tempdir, 'outside.txt') + + with tarfile.open(tar_filepath, 'w') as tar: + info = tarfile.TarInfo('.') + info.type = tarfile.DIRTYPE + tar.addfile(info) + + info = tarfile.TarInfo('redir') + info.type = tarfile.SYMTYPE + info.linkname = '.' + tar.addfile(info) + + info = tarfile.TarInfo('redir/up') + info.type = tarfile.SYMTYPE + info.linkname = '..' + tar.addfile(info) + + data = b'data\n' + info = tarfile.TarInfo('redir/up/outside.txt') + info.size = len(data) + tar.addfile(info, fileobj=io.BytesIO(data)) + + with pytest.raises(InstallationError): + untar_file(tar_filepath, extract_path) + + assert not os.path.exists(outside_path) + @pytest.mark.parametrize('args, expected', [ # Test the second containing the first. @@ -224,12 +341,35 @@ def test_unpack_tar_symlink_to_member_success(self): # CVE-2026-1703: a sibling whose name has the directory as a string # prefix must not be considered "within" it. (('parent/child', 'parent/childfoo'), False), + # A filesystem root contains everything below it. + (('/', '/usr/bin/pip'), True), + (('/', '/'), True), ]) def test_is_within_directory(args, expected): result = is_within_directory(*args) assert result == expected +@pytest.mark.skipif("sys.platform == 'win32'") +def test_is_within_directory_doubled_slash_root(): + # POSIX keeps a leading "//"; the containment check must too. + assert is_within_directory('//srv/env/bin', '//srv/env/bin/pip') + assert not is_within_directory('//srv/env/bin', '//srv/env/binfoo') + + +def test_is_within_directory_resolve_symlinks(tmpdir): + base = str(tmpdir) + inside = os.path.join(base, 'inside') + os.mkdir(inside) + if not hasattr(os, 'symlink'): + pytest.skip('os.symlink is unavailable') + os.symlink(base, os.path.join(inside, 'up')) + target = os.path.join(inside, 'up', 'outside.txt') + # Textually inside, but the symlink resolves to the parent. + assert is_within_directory(inside, target) + assert not is_within_directory(inside, target, resolve_symlinks=True) + + def _make_tar_zip_polyglot(path): """Write a valid .tar.gz with a valid ZIP appended (a polyglot file). @@ -283,3 +423,20 @@ def test_unpack_polyglot_ambiguous_rejected(tmpdir): _make_tar_zip_polyglot(archive) with pytest.raises(InstallationError): unpack_file(archive, os.path.join(str(tmpdir), 'out')) + +def test_tar_member_paths_terminates_on_doubled_slash(): + """posixpath.dirname("//") == "//"; collecting parents must not loop.""" + buf = io.BytesIO() + with tarfile.open(fileobj=buf, mode="w") as tar: + data = io.BytesIO(b"x") + info = tarfile.TarInfo("//abs/f.txt") + info.size = 1 + tar.addfile(info, fileobj=data) + buf.seek(0) + tar = tarfile.open(fileobj=buf) + try: + paths = _tar_member_paths(tar) + finally: + tar.close() + assert "//abs/f.txt" in paths + assert "//abs" in paths diff --git a/tests/unit/test_vcs.py b/tests/unit/test_vcs.py index 1008f19b6a..26e4f4f90d 100644 --- a/tests/unit/test_vcs.py +++ b/tests/unit/test_vcs.py @@ -52,7 +52,7 @@ def test_rev_options_repr(): # First check VCS-specific RevOptions behavior. (Bazaar, [], ['-r', '123'], {}), (Git, ['HEAD'], ['123'], {}), - (Mercurial, [], ['-r=123'], {}), + (Mercurial, [], ['--rev=123'], {}), (Subversion, [], ['-r', '123'], {}), # Test extra_args. For this, test using a single VersionControl class. (Git, ['HEAD', 'opt1', 'opt2'], ['123', 'opt1', 'opt2'], @@ -228,14 +228,25 @@ def test_git_get_revision_sha_unicode_separator(run_command_mock): def test_mercurial_rev_not_interpreted_as_option(): """ CVE-2023-5752: a malicious hg revision (e.g. ``--config=...``) must be - passed as a single ``-r=`` token so hg cannot parse it as an option. + passed as a single ``--rev=`` token so hg cannot parse it as an + option. """ rev = '--config=alias.update=!touch owned' args = Mercurial.get_base_rev_args(rev) - assert args == ['-r={}'.format(rev)] - # A single token means hg treats the whole thing as the value of -r. + assert args == ['--rev={}'.format(rev)] + # A single token means hg treats the whole thing as the value of --rev. assert len(args) == 1 - assert args[0].startswith('-r=') + assert args[0].startswith('--rev=') + + +def test_mercurial_rev_uses_long_option(): + """ + hg parses ``-r=`` as the revision ``=`` and fails, so the + revision must be passed with the long ``--rev=`` form (pypa/pip#12373). + """ + args = Mercurial.get_base_rev_args('0') + assert args == ['--rev=0'] + assert not args[0].startswith('-r') # The non-SVN backends all use the same get_netloc_and_auth(), so only test diff --git a/tests/unit/test_wheel.py b/tests/unit/test_wheel.py index df18c77b3f..1f6538ad3c 100644 --- a/tests/unit/test_wheel.py +++ b/tests/unit/test_wheel.py @@ -769,3 +769,18 @@ def test_script_within_dir_allows_doubled_slash_root(): def test_script_within_dir_rejects_other_drive(): # A script on a different drive is rejected, and doesn't raise assert wheel._script_within_dir("D:\\outside", "C:\\env\\bin") is False + + +@pytest.mark.skipif(WINDOWS, reason="POSIX filesystem root") +def test_script_within_dir_allows_filesystem_root(): + # A scripts directory at the filesystem root must still accept scripts, + # but never resolve a script to the root directory itself. + assert wheel._script_within_dir("pip", "/") is True + assert wheel._script_within_dir(".", "/") is False + assert wheel._script_within_dir("..", "/") is False + + +@pytest.mark.skipif(not WINDOWS, reason="drive letters only matter on Windows") +def test_script_within_dir_allows_drive_root(): + assert wheel._script_within_dir("pip", "C:\\") is True + assert wheel._script_within_dir(".", "C:\\") is False