Fix CWE-22 path traversal (Tar Slip) in dataset archive extraction

cornac/datasets/movielens.py

@@ -28,28 +28,28 @@
 MovieLens = namedtuple("MovieLens", ["url", "unzip", "path", "sep", "skip"])
 ML_DATASETS = {
     "100K": MovieLens(
-        "http://files.grouplens.org/datasets/movielens/ml-100k/u.data",
+        "https://files.grouplens.org/datasets/movielens/ml-100k/u.data",
         False,
         "ml-100k/u.data",
         "\t",
         0,
     ),
     "1M": MovieLens(
-        "http://files.grouplens.org/datasets/movielens/ml-1m.zip",
+        "https://files.grouplens.org/datasets/movielens/ml-1m.zip",
         True,
         "ml-1m/ratings.dat",
         "::",
         0,
     ),
     "10M": MovieLens(
-        "http://files.grouplens.org/datasets/movielens/ml-10m.zip",
+        "https://files.grouplens.org/datasets/movielens/ml-10m.zip",
         True,
         "ml-10M100K/ratings.dat",
         "::",
         0,
     ),
     "20M": MovieLens(
-        "http://files.grouplens.org/datasets/movielens/ml-20m.zip",
+        "https://files.grouplens.org/datasets/movielens/ml-20m.zip",
         True,
         "ml-20m/ratings.csv",
         ",",

cornac/utils/download.py

@@ -14,7 +14,7 @@
 # ============================================================================
 
 import os
-import shutil
+import sys
 import zipfile
 import tarfile
 from urllib import request
@@ -47,9 +47,38 @@ def report(chunk, chunksize, total):
         request.urlretrieve(url, fpath, reporthook=report)
 
 
-def _extract_archive(file_path, extract_path="."):
-    """Extracts an archive.
+def _safe_extract(archive, extract_path, extracted):
+    """Extract archive members one at a time, allowing only regular files and directories.
+    Blocks traversal paths, symlinks, hardlinks, FIFOs, and device files.
+    Appends each successfully written path to `extracted` for scoped cleanup on failure.
     """
+    extract_path = os.path.realpath(extract_path)
+    members = archive.getmembers() if hasattr(archive, "getmembers") else archive.infolist()
+    for member in members:
+        member_name = member.name if hasattr(member, "name") else member.filename
+        # Whitelist: allow only regular files and directories — blocks symlinks, hardlinks,
+        # FIFOs, character devices, block devices, and any other special TAR types
+        if hasattr(member, "isreg") and not (member.isreg() or member.isdir()):
+            raise ValueError(f"Blocked special member type in archive: {member_name}")
+        target = os.path.realpath(os.path.join(extract_path, member_name))
+        # Skip the root directory entry — extracting it can apply tar-owned permissions
+        if target == extract_path:
+            continue
+        if not target.startswith(extract_path + os.sep):
+            raise ValueError(f"Blocked path traversal attempt in archive: {member_name}")
+        # Record the target before writing so a partially-written file (failed mid-extract)
+        # is still cleaned up. Extract one member at a time so each write is validated
+        # against the live filesystem. Use filter="data" on Python 3.12+ as a defence-in-depth
+        # layer (safer than fully_trusted).
+        extracted.append(target)
+        if isinstance(archive, tarfile.TarFile) and sys.version_info >= (3, 12):
+            archive.extract(member, extract_path, filter="data")
+        else:
+            archive.extract(member, extract_path)
+
+
+def _extract_archive(file_path, extract_path="."):
+    """Extracts an archive."""
     for archive_type in ["zip", "tar"]:
         if archive_type == "zip":
             open_fn = zipfile.ZipFile
@@ -60,14 +89,21 @@ def _extract_archive(file_path, extract_path="."):
 
         if is_match_fn(file_path):
             with open_fn(file_path) as archive:
+                extracted = []
                 try:
-                    archive.extractall(extract_path)
-                except (tarfile.TarError, RuntimeError, KeyboardInterrupt):
-                    if os.path.exists(extract_path):
-                        if os.path.isfile(extract_path):
-                            os.remove(extract_path)
-                        else:
-                            shutil.rmtree(extract_path)
+                    _safe_extract(archive, extract_path, extracted)
+                except BaseException:
+                    # Any failure (corrupt archive, blocked member, interrupt, decompression
+                    # error, etc.) triggers cleanup, then the original exception is re-raised.
+                    # Remove only files written during this extraction — never wipe the full cache
+                    for path in reversed(extracted):
+                        try:
+                            if os.path.isfile(path) or os.path.islink(path):
+                                os.remove(path)
+                            elif os.path.isdir(path) and not os.listdir(path):
+                                os.rmdir(path)
+                        except OSError:
+                            pass
                     raise