Security fix for CVE-2026-13346
Co-Authored-by: Miro Hrončok <miro@hroncok.cz>
This commit is contained in:
parent
08dc5bf450
commit
ac655bc5de
2 changed files with 350 additions and 0 deletions
347
10dfb6b900.patch
Normal file
347
10dfb6b900.patch
Normal file
|
|
@ -0,0 +1,347 @@
|
|||
From 10dfb6b9005484578b386f64b9f36982e3dc6679 Mon Sep 17 00:00:00 2001
|
||||
From: Damian Shaw <damian.peter.shaw@gmail.com>
|
||||
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.
|
||||
---
|
||||
news/14110.bugfix.rst | 1 +
|
||||
src/pip/_internal/models/link.py | 63 ++++++++++---
|
||||
src/pip/_internal/network/download.py | 20 +++--
|
||||
src/pip/_internal/operations/prepare.py | 6 +-
|
||||
tests/unit/test_link.py | 113 +++++++++++++++++++++++-
|
||||
5 files changed, 182 insertions(+), 21 deletions(-)
|
||||
create mode 100644 news/14110.bugfix.rst
|
||||
|
||||
diff --git a/news/14110.bugfix.rst b/news/14110.bugfix.rst
|
||||
new file mode 100644
|
||||
index 0000000000..f7d4f78882
|
||||
--- /dev/null
|
||||
+++ b/news/14110.bugfix.rst
|
||||
@@ -0,0 +1 @@
|
||||
+Fix ``Link.filename`` decoding the URL path twice.
|
||||
diff --git a/src/pip/_internal/models/link.py b/src/pip/_internal/models/link.py
|
||||
index 0a09c66222..cbbe945c17 100644
|
||||
--- a/src/pip/_internal/models/link.py
|
||||
+++ b/src/pip/_internal/models/link.py
|
||||
@@ -13,6 +13,7 @@
|
||||
from typing import (
|
||||
Any,
|
||||
NamedTuple,
|
||||
+ NewType,
|
||||
)
|
||||
|
||||
from pip._internal.exceptions import InvalidEggFragment
|
||||
@@ -30,6 +31,49 @@
|
||||
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")
|
||||
@@ -424,18 +468,13 @@ def redacted_url(self) -> str:
|
||||
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 039b268878..6faafb5cb0 100644
|
||||
--- a/src/pip/_internal/network/download.py
|
||||
+++ b/src/pip/_internal/network/download.py
|
||||
@@ -19,7 +19,12 @@
|
||||
|
||||
from pip._internal.cli.progress_bars import BarType, get_download_progress_renderer
|
||||
from pip._internal.exceptions import IncompleteDownloadError, NetworkConnectionError
|
||||
-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
|
||||
@@ -121,11 +126,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:
|
||||
@@ -139,7 +147,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
|
||||
@@ -192,7 +200,9 @@ def __call__(self, link: Link, location: str) -> tuple[str, str]:
|
||||
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 afcc0376da..3b44403e0d 100644
|
||||
--- a/src/pip/_internal/operations/prepare.py
|
||||
+++ b/src/pip/_internal/operations/prepare.py
|
||||
@@ -29,7 +29,7 @@
|
||||
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, DirectUrl
|
||||
-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
|
||||
@@ -687,7 +687,7 @@ def save_linked_requirement(self, req: InstallRequirement) -> None:
|
||||
# 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 c49f8547ac..bc8cb8ab9b 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 @@ def test_repr(self, url: str, expected: str) -> None:
|
||||
("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 @@ def test_filename(self, url: str, expected: str) -> None:
|
||||
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
|
||||
|
|
@ -111,6 +111,9 @@ Patch: https://github.com/pypa/pip/commit/4c6d7471de.patch
|
|||
# https://github.com/pypa/pip/commit/09a03f6cfa (non-existing files removed)
|
||||
Patch: 09a03f6cfa.patch
|
||||
|
||||
# CVE-2026-13346: Link.filename double URL decode allows path traversal
|
||||
Patch: https://github.com/pypa/pip/commit/10dfb6b900.patch
|
||||
|
||||
# Remove -s from Python shebang - ensure that packages installed with pip
|
||||
# to user locations are seen by pip itself
|
||||
%undefine _py3_shebang_s
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue