Merge branch 'TrellixVulnTeam/master'

PR #1990

* TrellixVulnTeam/master:
  Update tar validation to also check for abspath and symlinks
  Adding tarfile member sanitization to extractall()

.changes/next-release/34727419579041-bugfix-tar-48521.json

@@ -0,0 +1,5 @@
+{
+  "type": "bugfix",
+  "category": "tar",
+  "description": "Validate tar extraction does not escape destination dir (#1990)"
+}

chalice/utils.py

@@ -235,8 +235,60 @@ def extract_zipfile(self, zipfile_path: str, unpack_dir: str) -> None:
 
     def extract_tarfile(self, tarfile_path: str, unpack_dir: str) -> None:
         with tarfile.open(tarfile_path, 'r:*') as tar:
+            # In Python 3.12+, there's a `filter` arg where passing a
+            # 'data' value will handle this behavior for us.  To support older
+            # versions of Python we handle this ourselves.  We can't hook
+            # into `extractall` directly so the idea is that we do a separate
+            # validation pass first to ensure there's no files that try
+            # to extract outside of the provided `unpack_dir`.  This is roughly
+            # based off of what's done in the `data_filter()` in Python 3.12.
+            self._validate_safe_extract(tar, unpack_dir)
             tar.extractall(unpack_dir)
 
+    def _validate_safe_extract(
+        self,
+        tar: tarfile.TarFile,
+        unpack_dir: str
+    ) -> None:
+        for member in tar:
+            self._validate_single_tar_member(member, unpack_dir)
+
+    def _validate_single_tar_member(
+        self,
+        member: tarfile.TarInfo,
+        unpack_dir: str
+    ) -> None:
+        name = member.name
+        dest_path = os.path.realpath(unpack_dir)
+        if name.startswith(('/', os.sep)):
+            name = member.path.lstrip('/' + os.sep)
+        if os.path.isabs(name):
+            raise RuntimeError(f"Absolute path in tarfile not allowed: {name}")
+        target_path = os.path.realpath(os.path.join(dest_path, name))
+        # Check we don't escape the destination dir, e.g `../../foo`
+        if os.path.commonpath([target_path, dest_path]) != dest_path:
+            raise RuntimeError(
+                f"Tar member outside destination dir: {target_path}")
+        # If we're dealing with a member that's some type of link, ensure
+        # it doesn't point to anything outside of the destination dir.
+        if member.islnk() or member.issym():
+            if os.path.abspath(member.linkname):
+                raise RuntimeError(f"Symlink to abspath: {member.linkname}")
+            if member.issym():
+                target_path = os.path.join(
+                    dest_path,
+                    os.path.dirname(name),
+                    member.linkname,
+                )
+            else:
+                target_path = os.path.join(
+                    dest_path,
+                    member.linkname)
+            target_path = os.path.realpath(target_path)
+            if os.path.commonpath([target_path, dest_path]) != dest_path:
+                raise RuntimeError(
+                    f"Symlink outside of dest dir: {target_path}")
+
     def directory_exists(self, path: str) -> bool:
         return os.path.isdir(path)
 

tests/functional/test_utils.py

@@ -2,6 +2,7 @@
 import json
 import os
 import io
+import tarfile
 
 import pytest
 
@@ -108,6 +109,31 @@ def test_remove_stage_from_deployed_values_no_file(tmpdir):
     assert not os.path.isfile(filename)
 
 
+def test_error_raised_on_tar_out_of_extract_dir(tmp_path, osutils):
+    filepath = tmp_path / 'badfile'
+    filepath.write_text('single file')
+    badtarpath = tmp_path / 'badtar.tar.gz'
+    extractdir = tmp_path / 'nest1' / 'nest2' / 'nest3'
+    with tarfile.open(badtarpath, 'w:gz') as tar:
+        tar.add(filepath, arcname='../../escaped-dir.txt')
+    with pytest.raises(RuntimeError):
+        osutils.extract_tarfile(str(badtarpath), extractdir)
+
+
+def test_error_raise_tar_symlink_out_of_extract_dir(tmp_path, osutils):
+    dir_with_symlink = tmp_path / 'nest1' / 'nest2'
+    dir_with_symlink.mkdir(parents=True, exist_ok=True)
+    outside_file = tmp_path / 'outside.txt'
+    outside_file.write_text('outside of dir')
+    symlink_file = dir_with_symlink / 'myfile.txt'
+    os.symlink(outside_file, symlink_file)
+    tarpath = dir_with_symlink / 'badtar.tar.gz'
+    with tarfile.open(tarpath, 'w:gz') as tar:
+        tar.add(symlink_file)
+    with pytest.raises(RuntimeError):
+        osutils.extract_tarfile(str(tarpath), dir_with_symlink)
+
+
 class TestOSUtils(object):
     def test_can_read_unicode(self, tmpdir, osutils):
         filename = str(tmpdir.join('file.txt'))