mirror of https://github.com/pypa/pip
Merge pull request #6418 from gzpan123/master
FIX #6413 pip install <url> allow directory traversal
This commit is contained in:
commit
5776ddd058
|
@ -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 ``..\\``.
|
|
@ -66,7 +66,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__)
|
||||
|
@ -1050,6 +1051,29 @@ def unpack_url(
|
|||
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, # type: Link
|
||||
session, # type: PipSession
|
||||
|
@ -1097,10 +1121,7 @@ def _download_http_url(
|
|||
# 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)
|
||||
|
|
|
@ -12,6 +12,7 @@ from mock import Mock, patch
|
|||
import pip
|
||||
from pip._internal.download import (
|
||||
CI_ENVIRONMENT_VARIABLES, MultiDomainBasicAuth, PipSession, SafeFileCache,
|
||||
_download_http_url, parse_content_disposition, sanitize_content_filename,
|
||||
unpack_file_url, unpack_http_url, url_to_path,
|
||||
)
|
||||
from pip._internal.exceptions import HashMismatch
|
||||
|
@ -199,6 +200,90 @@ 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,
|
||||
progress_bar='on',
|
||||
)
|
||||
# The file should be downloaded to download_dir.
|
||||
actual = os.listdir(download_dir)
|
||||
assert actual == ['out_dir_file']
|
||||
|
||||
|
||||
@pytest.mark.parametrize("url,win_expected,non_win_expected", [
|
||||
('file:tmp', 'tmp', 'tmp'),
|
||||
('file:c:/path/to/file', r'C:\path\to\file', 'c:/path/to/file'),
|
||||
|
|
Loading…
Reference in New Issue