Fix path traversal via metadata file_name in folder-based builders

src/datasets/packaged_modules/folder_based_builder/folder_based_builder.py

@@ -401,8 +401,54 @@ def set_feature(item, feature_path: _VisitPath):
                         )
                     elif len(feature_path) == 0:
                         if item is not None:
+                            # Guard against path traversal (CWE-22): a crafted `file_name` such as
+                            # "../../etc/passwd" or an absolute path must not be able to escape the
+                            # metadata file's directory and read arbitrary files on the host.
+                            #
+                            # The attacker-controlled `file_name` must be a plain relative path. In
+                            # particular it must not introduce an fsspec URL scheme: `file://` and
+                            # `local://` resolve to arbitrary *local* files, and any other scheme
+                            # would sidestep the containment check below. Legitimate reads from a
+                            # downloaded archive use a `zip://<file_name>::<container>` URL where the
+                            # scheme lives on `downloaded_metadata_dir` (the container), never on the
+                            # `file_name` value itself, so forbidding "://" here does not break them.
+                            if "://" in item:
+                                raise ValueError(
+                                    f"Invalid metadata file_name '{item}': `file_name` must be a relative path "
+                                    f"pointing inside the directory containing the metadata file. URL schemes "
+                                    f"(e.g. 'file://', 'local://') are not allowed."
+                                )
                             file_relpath = os.path.normpath(item).replace("\\", "/")
+                            if (
+                                os.path.isabs(item)
+                                or os.path.isabs(file_relpath)
+                                or file_relpath == ".."
+                                or file_relpath.startswith("../")
+                            ):
+                                raise ValueError(
+                                    f"Invalid metadata file_name '{item}': `file_name` must be a relative path "
+                                    f"pointing inside the directory containing the metadata file. Absolute paths "
+                                    f"and parent-directory ('..') traversal that escape the dataset directory are "
+                                    f"not allowed."
+                                )
                             item = os.path.join(downloaded_metadata_dir, file_relpath)
+                            # For local metadata directories, additionally resolve symlinks and confirm
+                            # containment. Skipped when the metadata dir is itself an fsspec URL (which
+                            # contains "://", e.g. "zip://...::..." from a downloaded archive) since
+                            # realpath is not scheme-aware and would corrupt the URL.
+                            if "://" not in downloaded_metadata_dir:
+                                real_root = os.path.realpath(downloaded_metadata_dir)
+                                real_path = os.path.realpath(item)
+                                try:
+                                    is_contained = os.path.commonpath([real_root, real_path]) == real_root
+                                except ValueError:
+                                    # commonpath raises ValueError for paths on different drives (Windows)
+                                    is_contained = False
+                                if not is_contained:
+                                    raise ValueError(
+                                        f"Invalid metadata file_name '{item}': the resolved path escapes the "
+                                        f"dataset directory containing the metadata file."
+                                    )
                     return item
 
                 for pa_metadata_table in self._read_metadata(downloaded_metadata_file, metadata_ext=metadata_ext):

tests/packaged_modules/test_folder_based_builder.py

@@ -1,4 +1,6 @@
 import importlib
+import json
+import os
 import shutil
 import textwrap
 
@@ -449,6 +451,121 @@ def test_data_files_with_wrong_metadata_file_name(cache_dir, tmp_path, auto_text
     assert all("additional_feature" not in example for _, example in generator)
 
 
+@pytest.mark.parametrize(
+    "malicious_file_name",
+    [
+        "../outside/secret.txt",  # relative traversal
+        "../../outside/secret.txt",  # deeper relative traversal
+        "subdir/../../outside/secret.txt",  # traversal hidden after a valid segment
+    ],
+)
+def test_data_files_with_metadata_path_traversal_is_rejected(cache_dir, tmp_path, auto_text_file, malicious_file_name):
+    # Regression test for https://github.com/huggingface/datasets/issues/8324
+    # A crafted `file_name` must not be able to escape the metadata file's directory.
+    secret_dir = tmp_path / "outside"
+    secret_dir.mkdir(parents=True, exist_ok=True)
+    shutil.copyfile(auto_text_file, secret_dir / "secret.txt")
+
+    data_dir = tmp_path / "data_dir_with_traversal_metadata"
+    data_dir.mkdir(parents=True, exist_ok=True)
+    shutil.copyfile(auto_text_file, data_dir / "file.txt")
+    metadata_filename = data_dir / "metadata.jsonl"
+    metadata = f'{{"file_name": {json.dumps(malicious_file_name)}, "additional_feature": "Malicious file"}}\n'
+    with open(metadata_filename, "w", encoding="utf-8") as f:
+        f.write(metadata)
+
+    data_files = DataFilesDict.from_patterns(get_data_patterns(str(data_dir)), data_dir.as_posix())
+    autofolder = DummyFolderBasedBuilder(data_files=data_files, cache_dir=cache_dir)
+    gen_kwargs = autofolder._split_generators(StreamingDownloadManager())[0].gen_kwargs
+    with pytest.raises(ValueError, match="Invalid metadata file_name"):
+        list(autofolder._generate_examples(**gen_kwargs))
+
+
+def test_data_files_with_metadata_absolute_path_is_rejected(cache_dir, tmp_path, auto_text_file):
+    # Regression test for https://github.com/huggingface/datasets/issues/8324
+    # An absolute `file_name` must not be able to read files outside the dataset directory.
+    secret_dir = tmp_path / "outside"
+    secret_dir.mkdir(parents=True, exist_ok=True)
+    secret_file = secret_dir / "secret.txt"
+    shutil.copyfile(auto_text_file, secret_file)
+
+    data_dir = tmp_path / "data_dir_with_absolute_metadata"
+    data_dir.mkdir(parents=True, exist_ok=True)
+    shutil.copyfile(auto_text_file, data_dir / "file.txt")
+    metadata_filename = data_dir / "metadata.jsonl"
+    metadata = f'{{"file_name": {json.dumps(str(secret_file))}, "additional_feature": "Malicious file"}}\n'
+    with open(metadata_filename, "w", encoding="utf-8") as f:
+        f.write(metadata)
+
+    data_files = DataFilesDict.from_patterns(get_data_patterns(str(data_dir)), data_dir.as_posix())
+    autofolder = DummyFolderBasedBuilder(data_files=data_files, cache_dir=cache_dir)
+    gen_kwargs = autofolder._split_generators(StreamingDownloadManager())[0].gen_kwargs
+    with pytest.raises(ValueError, match="Invalid metadata file_name"):
+        list(autofolder._generate_examples(**gen_kwargs))
+
+
+@pytest.mark.parametrize(
+    "malicious_file_name",
+    [
+        "file://../../outside/secret.txt",  # fsspec local scheme + traversal
+        "local://../../outside/secret.txt",  # fsspec local scheme + traversal
+        "file://../outside/secret.txt",  # scheme where os.path.normpath mangles the "../" away
+        "file:///etc/passwd",  # absolute-looking local scheme
+        "file://outside/secret.txt",  # scheme without traversal
+    ],
+)
+def test_data_files_with_metadata_url_scheme_is_rejected(cache_dir, tmp_path, auto_text_file, malicious_file_name):
+    # Regression test for the review on https://github.com/huggingface/datasets/pull/8325:
+    # fsspec schemes such as `file://` / `local://` resolve to local files and previously bypassed
+    # the containment check. A `file_name` carrying any URL scheme ("://") must be rejected.
+    secret_dir = tmp_path / "outside"
+    secret_dir.mkdir(parents=True, exist_ok=True)
+    shutil.copyfile(auto_text_file, secret_dir / "secret.txt")
+
+    data_dir = tmp_path / "data_dir_with_scheme_metadata"
+    data_dir.mkdir(parents=True, exist_ok=True)
+    shutil.copyfile(auto_text_file, data_dir / "file.txt")
+    metadata_filename = data_dir / "metadata.jsonl"
+    metadata = f'{{"file_name": {json.dumps(malicious_file_name)}, "additional_feature": "Malicious file"}}\n'
+    with open(metadata_filename, "w", encoding="utf-8") as f:
+        f.write(metadata)
+
+    data_files = DataFilesDict.from_patterns(get_data_patterns(str(data_dir)), data_dir.as_posix())
+    autofolder = DummyFolderBasedBuilder(data_files=data_files, cache_dir=cache_dir)
+    gen_kwargs = autofolder._split_generators(StreamingDownloadManager())[0].gen_kwargs
+    with pytest.raises(ValueError, match="Invalid metadata file_name"):
+        list(autofolder._generate_examples(**gen_kwargs))
+
+
+def test_data_files_with_metadata_legitimate_subdir_reference(cache_dir, tmp_path, auto_text_file):
+    # A legitimate `file_name` pointing to a file in a subdirectory of the metadata
+    # file's directory must keep working after the path traversal fix (#8324).
+    data_dir = tmp_path / "data_dir_with_subdir_metadata"
+    data_dir.mkdir(parents=True, exist_ok=True)
+    subdir = data_dir / "subdir"
+    subdir.mkdir(parents=True, exist_ok=True)
+    shutil.copyfile(auto_text_file, data_dir / "file.txt")
+    shutil.copyfile(auto_text_file, subdir / "file2.txt")
+    metadata_filename = data_dir / "metadata.jsonl"
+    metadata = textwrap.dedent(
+        """\
+        {"file_name": "file.txt", "additional_feature": "Same-dir file"}
+        {"file_name": "subdir/file2.txt", "additional_feature": "Subdir file"}
+        {"file_name": "./subdir/file2.txt", "additional_feature": "Subdir file with dot prefix"}
+        """
+    )
+    with open(metadata_filename, "w", encoding="utf-8") as f:
+        f.write(metadata)
+
+    data_files = DataFilesDict.from_patterns(get_data_patterns(str(data_dir)), data_dir.as_posix())
+    autofolder = DummyFolderBasedBuilder(data_files=data_files, cache_dir=cache_dir)
+    gen_kwargs = autofolder._split_generators(StreamingDownloadManager())[0].gen_kwargs
+    examples = [example for _, example in autofolder._generate_examples(**gen_kwargs)]
+    assert len(examples) == 3
+    assert all(example["base"] is not None for example in examples)
+    assert all(os.path.isfile(example["base"]) for example in examples)
+
+
 def test_data_files_with_custom_file_name_column_in_metadata_file(cache_dir, tmp_path, auto_text_file):
     data_dir = tmp_path / "data_dir_with_custom_file_name_metadata"
     data_dir.mkdir(parents=True, exist_ok=True)