rpms/python-pip
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions
python-pip/CVE-2026-13346.patch
Lumir Balhar 40c9b4419c Security fixes for CVE-2026-13346 and CVE-2026-8643
2026-08-26 08:52:14 +02:00

341 lines
13 KiB
Diff
Raw Permalink Blame History

From 6686b964762255a933e68939bb49710216ae7bb9 Mon Sep 17 00:00:00 2001
From: Damian Shaw <damian.peter.shaw@gmail.com>
Date: Fri, 14 Aug 2026 09:49:19 +0000
Subject: [PATCH 2/2] Fix Link.filename double URL decode - path traversal
(CVE-2026-13346)
Upstream PRs: https://github.com/pypa/pip/pull/14110
- Add PathComponent newtype to enforce single-component filenames
- Remove double urllib.parse.unquote() call in Link.filename
- Add join_within_directory() preventing path escape at download sites
- Update download.py and prepare.py call sites
- Backport test coverage from upstream commits 1, 3, 4 of PR #14110
Co-Authored-By: Lumir Balhar <lbalhar@redhat.com>
---
src/pip/_internal/models/link.py | 61 ++++++++++---
src/pip/_internal/network/download.py | 20 +++--
src/pip/_internal/operations/prepare.py | 6 +-
tests/unit/test_link.py | 113 +++++++++++++++++++++++-
4 files changed, 179 insertions(+), 21 deletions(-)
diff --git a/src/pip/_internal/models/link.py b/src/pip/_internal/models/link.py
index 200ec34..e6a0c87 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,47 @@ 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``.
+ """
+ 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 +465,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 2696642..fa71c75 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 67f9ee9..d260d15 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)
diff --git a/tests/unit/test_link.py b/tests/unit/test_link.py
index c49f854..bc8cb8a 100644
--- a/tests/unit/test_link.py
+++ b/tests/unit/test_link.py
@@ -1,9 +1,17 @@
from __future__ import annotations
+import os
+import posixpath
+
import pytest
from pip._internal.exceptions import InvalidEggFragment, PipError
-from pip._internal.models.link import Link, links_equivalent
+from pip._internal.models.link import (
+ Link,
+ as_path_component,
+ join_within_directory,
+ links_equivalent,
+)
from pip._internal.utils.hashes import Hashes
@@ -29,6 +37,13 @@ class TestLink:
("https://example.com/path/page.html", "page.html"),
# Test a quoted character.
("https://example.com/path/page%231.html", "page#1.html"),
+ # A doubly-encoded separator must stay encoded: the path is decoded
+ # exactly once, so the file name keeps its literal "%2F" instead of
+ # collapsing into a "/".
+ (
+ "https://example.com/a%252Fb.whl",
+ "a%2Fb.whl",
+ ),
(
"http://yo/myproject-1.0%2Bfoobar.0-py2.py3-none-any.whl",
"myproject-1.0+foobar.0-py2.py3-none-any.whl",
@@ -49,6 +64,52 @@ class TestLink:
link = Link(url)
assert link.filename == expected
+ @pytest.mark.parametrize(
+ "url",
+ [
+ "https://example.com/a%252Fb.whl",
+ "https://example.com/%252e%252e%252fb.whl",
+ ],
+ )
+ def test_filename_decoded_once_stays_single_component(self, url: str) -> None:
+ # The path is decoded exactly once, so an encoded separator stays
+ # encoded and the file name remains a single path component rather
+ # than collapsing into a "/"-separated path.
+ filename = Link(url).filename
+ assert not posixpath.isabs(filename)
+ assert posixpath.basename(filename) == filename
+
+ @pytest.mark.parametrize(
+ "url",
+ [
+ "https://example.com/..",
+ "https://example.com/.",
+ "https://example.com/foo/%2e%2e",
+ ],
+ )
+ def test_filename_parent_reference_falls_back_to_netloc(self, url: str) -> None:
+ # A path that is only a "." or ".." reference has no usable file name,
+ # so filename falls back to the netloc rather than handing back a
+ # traversal component that could escape a download directory.
+ assert Link(url).filename == "example.com"
+
+ @pytest.mark.parametrize(
+ "url",
+ [
+ # A path-less URL whose authority looks like a traversal: the netloc
+ # fallback must still reduce to a single path component.
+ "http://..\\..\\..\\evil.whl",
+ "http://../",
+ "http://..",
+ ],
+ )
+ def test_filename_is_always_a_path_component(self, url: str) -> None:
+ # filename must never carry a separator or parent reference, so joining
+ # it onto a directory can never escape that directory.
+ name = Link(url).filename
+ assert os.path.basename(name) == name
+ assert name not in (os.curdir, os.pardir)
+
def test_splitext(self) -> None:
assert ("wheel", ".whl") == Link("http://yo/wheel.whl").splitext()
@@ -244,3 +305,53 @@ def test_links_equivalent(url1: str, url2: str) -> None:
)
def test_links_equivalent_false(url1: str, url2: str) -> None:
assert not links_equivalent(Link(url1), Link(url2))
+
+
+@pytest.mark.parametrize(
+ "name",
+ [
+ "wheel.whl",
+ "myproject-1.0+foobar.0-py2.py3-none-any.whl",
+ # A literal "%2F" is a normal file name, not a separator.
+ "a%2Fb.whl",
+ ],
+)
+def test_as_path_component_keeps_plain_name(name: str) -> None:
+ assert as_path_component(name) == name
+
+
+@pytest.mark.parametrize(
+ "name",
+ [
+ os.path.join(os.sep, "abs", "pkg.whl"),
+ os.path.join("..", "pkg.whl"),
+ os.path.join("nested", "pkg.whl"),
+ ],
+)
+def test_as_path_component_reduces_to_basename(name: str) -> None:
+ # A name carrying directory components is reduced to its basename, so the
+ # result always stays inside the directory it is later joined onto.
+ assert as_path_component(name) == os.path.basename(name)
+
+
+@pytest.mark.parametrize("name", ["", ".", "..", "/", os.path.join("sub", "..")])
+def test_as_path_component_rejects_empty_or_parent_reference(name: str) -> None:
+ with pytest.raises(ValueError):
+ as_path_component(name)
+
+
+@pytest.mark.parametrize(
+ "name",
+ [
+ "pkg.whl",
+ # A literal "%2F" is a normal file name, not a separator.
+ "a%2Fb.whl",
+ ],
+)
+def test_join_within_directory_stays_inside(name: str) -> None:
+ # The component is joined onto the directory as its final element, so the
+ # result stays inside the directory.
+ directory = os.path.join("base", "downloads")
+ joined = join_within_directory(directory, as_path_component(name))
+ assert joined == os.path.join(directory, name)
+ assert os.path.basename(joined) == name
--
2.55.0
Reference in a new issue View git blame Copy permalink