Address review comments

news/3907.bugfix

-Abort the installation process and raise an exception if one of the tar/zip file will be placed outside of target location causing security issue.
\ No newline at end of file
+Abort installation if any archive contains a file which would be placed
+outside the extraction location.

src/pip/_internal/utils/unpacking.py

@@ -120,11 +120,11 @@ def unzip_file(filename, location, flatten=True):
             fn = os.path.join(location, fn)
             dir = os.path.dirname(fn)
             if not is_within_directory(location, fn):
-                raise InstallationError(
-                    'The zip file (%s) has a file (%s) trying to install '
-                    'outside target directory (%s)' %
-                    (filename, fn, location)
+                message = (
+                    'The zip file ({}) has a file ({}) trying to install '
+                    'outside target directory ({})'
                 )
+                raise InstallationError(message.format(filename, fn, location))
             if fn.endswith('/') or fn.endswith('\\'):
                 # A directory
                 ensure_dir(fn)
@@ -185,10 +185,12 @@ def untar_file(filename, location):
                 fn = split_leading_dir(fn)[1]  # type: ignore
             path = os.path.join(location, fn)
             if not is_within_directory(location, path):
+                message = (
+                    'The tar file ({}) has a file ({}) trying to install '
+                    'outside target directory ({})'
+                )
                 raise InstallationError(
-                    'The tar file (%s) has a file (%s) trying to install '
-                    'outside target directory (%s)' %
-                    (filename, path, location)
+                    message.format(filename, path, location)
                 )
             if member.isdir():
                 ensure_dir(path)

tests/unit/test_utils_unpacking.py

@@ -126,13 +126,11 @@ class TestUnpackArchives(object):
         Test unpacking a *.zip with file containing .. path
         and expect exception
         """
-        test_zip = self.make_zip_file('test_zip.zip',
-                                      ['regular_file.txt',
-                                       os.path.join('..', 'outside_file.txt')])
-        with pytest.raises(
-                InstallationError,
-                match=r'.*trying to install outside target directory.*'):
+        files = ['regular_file.txt', os.path.join('..', 'outside_file.txt')]
+        test_zip = self.make_zip_file('test_zip.zip', files)
+        with pytest.raises(InstallationError) as e:
             unzip_file(test_zip, self.tempdir)
+        assert 'trying to install outside target directory' in str(e.value)
 
     def test_unpack_zip_success(self):
         """
@@ -140,11 +138,12 @@ class TestUnpackArchives(object):
         no file will be installed outside target directory after unpack
         so no exception raised
         """
-        test_zip = self.make_zip_file(
-            'test_zip.zip',
-            ['regular_file1.txt',
-             os.path.join('dir', 'dir_file1.txt'),
-             os.path.join('dir', '..', 'dir_file2.txt')])
+        files = [
+            'regular_file1.txt',
+            os.path.join('dir', 'dir_file1.txt'),
+            os.path.join('dir', '..', 'dir_file2.txt'),
+        ]
+        test_zip = self.make_zip_file('test_zip.zip', files)
         unzip_file(test_zip, self.tempdir)
 
     def test_unpack_tar_failure(self):
@@ -152,13 +151,11 @@ class TestUnpackArchives(object):
         Test unpacking a *.tar with file containing .. path
         and expect exception
         """
-        test_tar = self.make_tar_file('test_tar.tar',
-                                      ['regular_file.txt',
-                                       os.path.join('..', 'outside_file.txt')])
-        with pytest.raises(
-                InstallationError,
-                match=r'.*trying to install outside target directory.*'):
+        files = ['regular_file.txt', os.path.join('..', 'outside_file.txt')]
+        test_tar = self.make_tar_file('test_tar.tar', files)
+        with pytest.raises(InstallationError) as e:
             untar_file(test_tar, self.tempdir)
+        assert 'trying to install outside target directory' in str(e.value)
 
     def test_unpack_tar_success(self):
         """
@@ -166,11 +163,12 @@ class TestUnpackArchives(object):
         no file will be installed outside target directory after unpack
         so no exception raised
         """
-        test_tar = self.make_tar_file(
-            'test_tar.tar',
-            ['regular_file1.txt',
-             os.path.join('dir', 'dir_file1.txt'),
-             os.path.join('dir', '..', 'dir_file2.txt')])
+        files = [
+            'regular_file1.txt',
+            os.path.join('dir', 'dir_file1.txt'),
+            os.path.join('dir', '..', 'dir_file2.txt'),
+        ]
+        test_tar = self.make_tar_file('test_tar.tar', files)
         untar_file(test_tar, self.tempdir)