From 10dfb6b9005484578b386f64b9f36982e3dc6679 Mon Sep 17 00:00:00 2001 From: Damian Shaw Date: Tue, 30 Jun 2026 21:52:39 -0400 Subject: [PATCH] Fix Link.filename decoding URL path twice (#14110) Link already percent-decodes the URL path into `self._path`, but `Link.filename` decoded the basename again, so a doubly-encoded separator was decoded twice: `%252F` became `%2F` in `__init__`, then `/` in `filename`, turning the single component `a%2Fb.whl` into `a/b.whl`. Drop the second decode, and add a `join_within_directory` helper so the download-path joins treat the name as a single path component. CVE: CVE-2026-13346 Upstream-Status: Backport [https://github.com/pypa/pip/commit/10dfb6b9005484578b386f64b9f36982e3dc6679] Backport Changes: - Omit news/14110.bugfix.rst and tests/unit/test_link.py because these paths are absent from the pip 26.0.1 PyPI sdist used by this recipe. All runtime-source changes are unchanged from upstream. (cherry picked from commit 10dfb6b9005484578b386f64b9f36982e3dc6679) Signed-off-by: Hetvi Thakar --- src/pip/_internal/models/link.py | 63 ++++++++++++++++++++----- src/pip/_internal/network/download.py | 20 ++++++-- src/pip/_internal/operations/prepare.py | 6 +-- 3 files changed, 69 insertions(+), 20 deletions(-) diff --git a/src/pip/_internal/models/link.py b/src/pip/_internal/models/link.py index 200ec34c5..1a6439873 100644 --- a/src/pip/_internal/models/link.py +++ b/src/pip/_internal/models/link.py @@ -14,6 +14,7 @@ from dataclasses import dataclass from typing import ( Any, NamedTuple, + NewType, ) from pip._internal.exceptions import InvalidEggFragment @@ -31,6 +32,49 @@ from pip._internal.utils.urls import path_to_url, url_to_path logger = logging.getLogger(__name__) +# A single path component: percent-decoded once and reduced to a basename, so it +# contains no path separator and is not a ``.`` or ``..`` reference. The empty +# string means "no component". +PathComponent = NewType("PathComponent", str) + + +def _to_path_component(name: str) -> PathComponent: + """Reduce ``name`` to a single path component, or ``""`` if it has none. + + ``os.path.basename`` drops any directory part, drive letter, or separator; + a ``.``, ``..``, or empty result is not a component and becomes ``""``. + """ + name = os.path.basename(name) + if name in ("", os.curdir, os.pardir): + return PathComponent("") + + return PathComponent(name) + + +def as_path_component(name: str) -> PathComponent: + """Like ``_to_path_component`` but reject the empty result. + + Use where a file is about to be written, so a missing name is an error + rather than a silent fallback to the directory itself. + """ + component = _to_path_component(name) + if not component: + raise ValueError(f"Unexpected file name derived from URL: {name!r}") + + return component + + +def join_within_directory(directory: str, component: PathComponent) -> str: + """Join a single path ``component`` onto ``directory``. + + ``component`` is a :data:`PathComponent`, so by type it has no separator and + is not a ``.`` or ``..`` reference; the result can never escape ``directory``. + Requiring ``PathComponent`` rather than ``str`` lets the type checker enforce + at the call site that the name was reduced to a safe component beforehand. + """ + return os.path.join(directory, component) + + # Order matters, earlier hashes have a precedence over later hashes for what # we will pick to use. _SUPPORTED_HASHES = ("sha512", "sha384", "sha256", "sha224", "sha1", "md5") @@ -423,18 +467,13 @@ class Link: return redact_auth_from_url(self.url) @property - def filename(self) -> str: - path = self.path.rstrip("/") - name = posixpath.basename(path) - if not name: - # Make sure we don't leak auth information if the netloc - # includes a username and password. - netloc, user_pass = split_auth_from_netloc(self.netloc) - return netloc - - name = urllib.parse.unquote(name) - assert name, f"URL {self._url!r} produced no filename" - return name + def filename(self) -> PathComponent: + name = _to_path_component(posixpath.basename(self.path.rstrip("/"))) + if name: + return name + + # No component in the path; fall back to the netloc, dropping any auth. + return _to_path_component(split_auth_from_netloc(self.netloc)[0]) @property def file_path(self) -> str: diff --git a/src/pip/_internal/network/download.py b/src/pip/_internal/network/download.py index 26966423f..fa71c75a3 100644 --- a/src/pip/_internal/network/download.py +++ b/src/pip/_internal/network/download.py @@ -20,7 +20,12 @@ from pip._vendor.urllib3.exceptions import ReadTimeoutError from pip._internal.cli.progress_bars import BarType, get_download_progress_renderer from pip._internal.exceptions import IncompleteDownloadError, NetworkConnectionError from pip._internal.models.index import PyPI -from pip._internal.models.link import Link +from pip._internal.models.link import ( + Link, + PathComponent, + as_path_component, + join_within_directory, +) from pip._internal.network.cache import SafeFileCache, is_from_cache from pip._internal.network.session import CacheControlAdapter, PipSession from pip._internal.network.utils import HEADERS, raise_for_status, response_chunks @@ -117,11 +122,14 @@ def parse_content_disposition(content_disposition: str, default_filename: str) - return filename or default_filename -def _get_http_response_filename(resp: Response, link: Link) -> str: +def _get_http_response_filename(resp: Response, link: Link) -> PathComponent: """Get an ideal filename from the given HTTP response, falling back to the link filename if not provided. + + The result is validated as a single path component, so it can be joined onto + a download directory without escaping it. """ - filename = link.filename # fallback + filename: str = link.filename # fallback # Have a look at the Content-Disposition header for a better guess content_disposition = resp.headers.get("content-disposition") if content_disposition: @@ -135,7 +143,7 @@ def _get_http_response_filename(resp: Response, link: Link) -> str: ext = os.path.splitext(resp.url)[1] if ext: filename += ext - return filename + return as_path_component(filename) @dataclass @@ -188,7 +196,9 @@ class Downloader: resp = self._http_get(link) download_size = _get_http_response_size(resp) - filepath = os.path.join(location, _get_http_response_filename(resp, link)) + filepath = join_within_directory( + location, _get_http_response_filename(resp, link) + ) with open(filepath, "wb") as content_file: download = _FileDownload(link, content_file, download_size) self._process_response(download, resp) diff --git a/src/pip/_internal/operations/prepare.py b/src/pip/_internal/operations/prepare.py index 67f9ee950..d260d15a2 100644 --- a/src/pip/_internal/operations/prepare.py +++ b/src/pip/_internal/operations/prepare.py @@ -29,7 +29,7 @@ from pip._internal.exceptions import ( from pip._internal.index.package_finder import PackageFinder from pip._internal.metadata import BaseDistribution, get_metadata_distribution from pip._internal.models.direct_url import ArchiveInfo -from pip._internal.models.link import Link +from pip._internal.models.link import Link, join_within_directory from pip._internal.models.wheel import Wheel from pip._internal.network.download import Downloader from pip._internal.network.lazy_wheel import ( @@ -201,7 +201,7 @@ def _check_download_dir( """Check download_dir for previously downloaded file with correct hash If a correct file is found return its path else None """ - download_path = os.path.join(download_dir, link.filename) + download_path = join_within_directory(download_dir, link.filename) if not os.path.exists(download_path): return None @@ -683,7 +683,7 @@ class RequirementPreparer: # No distribution was downloaded for this requirement. return - download_location = os.path.join(self.download_dir, link.filename) + download_location = join_within_directory(self.download_dir, link.filename) if not os.path.exists(download_location): shutil.copy(req.local_file_path, download_location) download_path = display_path(download_location) -- 2.35.6