MNT Fix path traversal issue in archive extraction (#4924)

The `_is_attempting_path_traversal` function did not properly handle
absolute paths, allowing a maliciously crafted archive to write files
outside of the intended extraction directory.

This patch fixes the issue by:
- Adding a check to explicitly disallow absolute paths in archive
members.
- Improving the relative path traversal check to ensure that the
resolved path of an archive member is always within the intended output
directory.

b/431292024

---------

Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>

Author: vitaliset

src/clusterfuzz/_internal/system/archive.py

@@ -65,19 +65,20 @@ def _is_attempting_path_traversal(archive_name: StrBytesPathLike,
       Whether there is a path traversal attempt
   """
   output_dir = os.path.realpath(output_dir)
-  absolute_file_path = os.path.join(output_dir, os.path.normpath(filename))
-  real_file_path = os.path.realpath(absolute_file_path)
-
-  if real_file_path == output_dir:
-    # Workaround for https://bugs.python.org/issue28488.
-    # Ignore directories named '.'.
-    return False
-
-  if real_file_path != absolute_file_path:
+  if os.path.isabs(filename):
     logs.error('Directory traversal attempted while unpacking archive %s '
-               '(file path=%s, actual file path=%s). Aborting.' %
-               (archive_name, absolute_file_path, real_file_path))
+               '(absolute file path=%s). Aborting.' % (archive_name, filename))
     return True
+
+  real_file_path = os.path.realpath(os.path.join(output_dir, filename))
+  if (not real_file_path.startswith(output_dir + os.sep) and
+      real_file_path != output_dir):
+    logs.error(
+        'Directory traversal attempted while unpacking archive %s '
+        '(file path=%s, real file path=%s). Aborting.' %
+        (archive_name, os.path.join(output_dir, filename), real_file_path))
+    return True
+
   return False
 
 

src/clusterfuzz/_internal/tests/core/system/archive_test.py

@@ -12,7 +12,9 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 """archive tests."""
+import io
 import os
+import tarfile
 import tempfile
 import unittest
 
@@ -51,6 +53,29 @@ def test_file_list(self):
           [f.name for f in reader.list_members()],
           ["archive_dir", "archive_dir/bye", "archive_dir/hi"])
 
+  def test_unpack_absolute_path_traversal(self):
+    """Test that unpacking an archive with an absolute path fails."""
+    # Create a temporary TAR archive file on disk.
+    with tempfile.NamedTemporaryFile(suffix='.tar') as tmp_tar_file:
+      malicious_archive_path = tmp_tar_file.name
+
+      # Create a malicious archive with an absolute path payload.
+      with tarfile.open(malicious_archive_path, 'w') as tar:
+        file_data = b'malicious content'
+        # Creating a TarInfo object with an absolute path.
+        tarinfo = tarfile.TarInfo(name='/tmp/pwned')
+        tarinfo.size = len(file_data)
+        tar.addfile(tarinfo, io.BytesIO(file_data))
+
+      output_directory = tempfile.mkdtemp()
+
+      # The function should return False, indicating an error occurred.
+      with archive.open(malicious_archive_path) as reader:
+        result = reader.extract_all(output_directory, trusted=False)
+        self.assertFalse(result)
+
+      shell.remove_directory(output_directory)
+
 
 class ArchiveReaderTest(unittest.TestCase):
   """Tests for the archive.iterator function."""