#2 Patch for pip install <url> allow directory traversal, leading to arbitrary file write
Merged 3 years ago by lbalhar. Opened 3 years ago by torsava.
rpms/ torsava/python-pip-epel epel7  into  epel7

@@ -0,0 +1,122 @@ 

+ From 7917dbda14ef64a5e7fdea48383a266577484ac8 Mon Sep 17 00:00:00 2001

+ From: Tomas Orsava <torsava@redhat.com>

+ Date: Wed, 19 Aug 2020 12:51:16 +0200

+ Subject: [PATCH 2/2] FIX #6413 pip install <url> allow directory traversal

+  (tests)

+ 

+ ---

+  tests/unit/test_download.py | 85 +++++++++++++++++++++++++++++++++++++

+  1 file changed, 85 insertions(+)

+ 

+ diff --git a/tests/unit/test_download.py b/tests/unit/test_download.py

+ index ee4b11c..15f99ec 100644

+ --- a/tests/unit/test_download.py

+ +++ b/tests/unit/test_download.py

+ @@ -1,5 +1,6 @@

+  import hashlib

+  import os

+ +import sys

+  from io import BytesIO

+  from shutil import rmtree, copy

+  from tempfile import mkdtemp

+ @@ -13,6 +14,7 @@ import pip

+  from pip.exceptions import HashMismatch

+  from pip.download import (

+      PipSession, SafeFileCache, path_to_url, unpack_http_url, url_to_path,

+ +    _download_http_url, parse_content_disposition, sanitize_content_filename,

+      unpack_file_url,

+  )

+  from pip.index import Link

+ @@ -123,6 +125,89 @@ def test_unpack_http_url_bad_downloaded_checksum(mock_unpack_file):

+          rmtree(download_dir)

+  

+  

+ +@pytest.mark.parametrize("filename, expected", [

+ +    ('dir/file', 'file'),

+ +    ('../file', 'file'),

+ +    ('../../file', 'file'),

+ +    ('../', ''),

+ +    ('../..', '..'),

+ +    ('/', ''),

+ +])

+ +def test_sanitize_content_filename(filename, expected):

+ +    """

+ +    Test inputs where the result is the same for Windows and non-Windows.

+ +    """

+ +    assert sanitize_content_filename(filename) == expected

+ +

+ +

+ +@pytest.mark.parametrize("filename, win_expected, non_win_expected", [

+ +    ('dir\\file', 'file', 'dir\\file'),

+ +    ('..\\file', 'file', '..\\file'),

+ +    ('..\\..\\file', 'file', '..\\..\\file'),

+ +    ('..\\', '', '..\\'),

+ +    ('..\\..', '..', '..\\..'),

+ +    ('\\', '', '\\'),

+ +])

+ +def test_sanitize_content_filename__platform_dependent(

+ +    filename,

+ +    win_expected,

+ +    non_win_expected

+ +):

+ +    """

+ +    Test inputs where the result is different for Windows and non-Windows.

+ +    """

+ +    if sys.platform == 'win32':

+ +        expected = win_expected

+ +    else:

+ +        expected = non_win_expected

+ +    assert sanitize_content_filename(filename) == expected

+ +

+ +

+ +@pytest.mark.parametrize("content_disposition, default_filename, expected", [

+ +    ('attachment;filename="../file"', 'df', 'file'),

+ +])

+ +def test_parse_content_disposition(

+ +    content_disposition,

+ +    default_filename,

+ +    expected

+ +):

+ +    actual = parse_content_disposition(content_disposition, default_filename)

+ +    assert actual == expected

+ +

+ +

+ +def test_download_http_url__no_directory_traversal(tmpdir):

+ +    """

+ +    Test that directory traversal doesn't happen on download when the

+ +    Content-Disposition header contains a filename with a ".." path part.

+ +    """

+ +    mock_url = 'http://www.example.com/whatever.tgz'

+ +    contents = b'downloaded'

+ +    link = Link(mock_url)

+ +

+ +    session = Mock()

+ +    resp = MockResponse(contents)

+ +    resp.url = mock_url

+ +    resp.headers = {

+ +        # Set the content-type to a random value to prevent

+ +        # mimetypes.guess_extension from guessing the extension.

+ +        'content-type': 'random',

+ +        'content-disposition': 'attachment;filename="../out_dir_file"'

+ +    }

+ +    session.get.return_value = resp

+ +

+ +    download_dir = tmpdir.join('download')

+ +    os.mkdir(download_dir)

+ +    file_path, content_type = _download_http_url(

+ +        link,

+ +        session,

+ +        download_dir,

+ +        hashes=None,

+ +    )

+ +    # The file should be downloaded to download_dir.

+ +    actual = os.listdir(download_dir)

+ +    assert actual == ['out_dir_file']

+ +

+ +

+  @pytest.mark.skipif("sys.platform == 'win32'")

+  def test_path_to_url_unix():

+      assert path_to_url('/tmp/file') == 'file:///tmp/file'

+ -- 

+ 2.25.4

+ 

@@ -0,0 +1,79 @@ 

+ From 8044d9f2fbcb09f09a62b26ac1d8a134976bb2ac Mon Sep 17 00:00:00 2001

+ From: gzpan123 <gzpan123@gmail.com>

+ Date: Wed, 17 Apr 2019 21:25:45 +0800

+ Subject: [PATCH 1/2] FIX #6413 pip install <url> allow directory traversal

+ 

+ ---

+  news/6413.bugfix |  3 +++

+  pip/download.py  | 31 ++++++++++++++++++++++++++-----

+  2 files changed, 29 insertions(+), 5 deletions(-)

+  create mode 100644 news/6413.bugfix

+ 

+ diff --git a/news/6413.bugfix b/news/6413.bugfix

+ new file mode 100644

+ index 0000000..68d0a72

+ --- /dev/null

+ +++ b/news/6413.bugfix

+ @@ -0,0 +1,3 @@

+ +Prevent ``pip install <url>`` from permitting directory traversal if e.g.

+ +a malicious server sends a ``Content-Disposition`` header with a filename

+ +containing ``../`` or ``..\\``.

+ diff --git a/pip/download.py b/pip/download.py

+ index 039e55a..b3d169b 100644

+ --- a/pip/download.py

+ +++ b/pip/download.py

+ @@ -54,7 +54,8 @@ __all__ = ['get_file_content',

+             'is_url', 'url_to_path', 'path_to_url',

+             'is_archive_file', 'unpack_vcs_link',

+             'unpack_file_url', 'is_vcs_url', 'is_file_url',

+ -           'unpack_http_url', 'unpack_url']

+ +           'unpack_http_url', 'unpack_url',

+ +           'parse_content_disposition', 'sanitize_content_filename']

+  

+  

+  logger = logging.getLogger(__name__)

+ @@ -824,6 +825,29 @@ def unpack_url(link, location, download_dir=None,

+          write_delete_marker_file(location)

+  

+  

+ +def sanitize_content_filename(filename):

+ +    # type: (str) -> str

+ +    """

+ +    Sanitize the "filename" value from a Content-Disposition header.

+ +    """

+ +    return os.path.basename(filename)

+ +

+ +

+ +def parse_content_disposition(content_disposition, default_filename):

+ +    # type: (str, str) -> str

+ +    """

+ +    Parse the "filename" value from a Content-Disposition header, and

+ +    return the default filename if the result is empty.

+ +    """

+ +    _type, params = cgi.parse_header(content_disposition)

+ +    filename = params.get('filename')

+ +    if filename:

+ +        # We need to sanitize the filename to prevent directory traversal

+ +        # in case the filename contains ".." path parts.

+ +        filename = sanitize_content_filename(filename)

+ +    return filename or default_filename

+ +

+ +

+  def _download_http_url(link, session, temp_dir, hashes):

+      """Download link url into temp_dir using provided session"""

+      target_url = link.url.split('#', 1)[0]

+ @@ -864,10 +888,7 @@ def _download_http_url(link, session, temp_dir, hashes):

+      # Have a look at the Content-Disposition header for a better guess

+      content_disposition = resp.headers.get('content-disposition')

+      if content_disposition:

+ -        type, params = cgi.parse_header(content_disposition)

+ -        # We use ``or`` here because we don't want to use an "empty" value

+ -        # from the filename param.

+ -        filename = params.get('filename') or filename

+ +        filename = parse_content_disposition(content_disposition, filename)

+      ext = splitext(filename)[1]

+      if not ext:

+          ext = mimetypes.guess_extension(content_type)

+ -- 

+ 2.25.4

+ 

file modified
+18 -1
@@ -14,7 +14,7 @@ 

  

  Name:           python-%{srcname}-epel

  Version:        8.1.2

- Release:        13%{?dist}

+ Release:        14%{?dist}

  Summary:        A tool for installing and managing Python packages

  

  # We bundle a lot of libraries with pip, which itself is under MIT license.
@@ -74,6 +74,14 @@ 

  # https://github.com/psf/requests/pull/4851

  Patch4:         CVE-2018-18074.patch

  

+ # Patch for pip install <url> allow directory traversal, leading to arbitrary file write

+ # - Upstream PR: https://github.com/pypa/pip/pull/6418/files

+ # - Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1868016

+ # Patch9 fixes the issue

+ # Patch10 adds unit tests for the issue

+ Patch9:         pip-directory-traversal-security-issue.patch

+ Patch10:        pip-directory-traversal-security-issue-tests.patch

+ 

  %description

  Pip is a replacement for easy_install.  It uses mostly the

  same techniques for finding packages, so packages that were made
@@ -249,6 +257,11 @@ 

  %patch4 -p1

  popd

  

+ %patch9 -p1

+ %if 0%{?with_tests}

+ %patch10 -p1

+ %endif

+ 

  sed -i '1d' pip/__init__.py

  

  
@@ -399,6 +412,10 @@ 

  %endif # with_python3_other

  

  %changelog

+ * Wed Sep 02 2020 Tomas Orsava <torsava@redhat.com> - 8.1.2-14

+ - Patch for pip install <url> allow directory traversal, leading to arbitrary file write

+ Resolves: rhbz#1868137

+ 

  * Thu Jan 30 2020 Fedora Release Engineering <releng@fedoraproject.org> - 8.1.2-13

  - Rebuilt for https://fedoraproject.org/wiki/Fedora_32_Mass_Rebuild

  

Resolves: rhbz#1868137

Verified to work using reproducer and test suite: https://bugzilla.redhat.com/show_bug.cgi?id=1868137#c2

ship it

Thank you! I'm gonna ship it.

Pull-Request has been merged by lbalhar

3 years ago