Skip to content

Fix path traversal in download_external_data containment guard - #2486

Open
lollinng wants to merge 1 commit into
basetenlabs:mainfrom
lollinng:fix/external-data-path-traversal
Open

Fix path traversal in download_external_data containment guard#2486
lollinng wants to merge 1 commit into
basetenlabs:mainfrom
lollinng:fix/external-data-path-traversal

Conversation

@lollinng

@lollinng lollinng commented Jun 5, 2026

Copy link
Copy Markdown

Problem (path traversal)

download_external_data validates each external-data item before downloading:

path = data_dir / item.local_data_path
if data_dir not in path.parents:
    raise ValueError("Local data path of external data cannot point to outside data directory")

pathlib does not normalize .., so for local_data_path = "../evil.bin":

  • path = data_dir / "../evil.bin"Path("/data/../evil.bin")
  • path.parents still contains data_dir (/data), so the guard passes.

But the actual download paths resolve first — _download_external_data_using_b10cp and _download_external_data_using_requests both compute (data_dir / item.local_data_path).resolve() — which for ../evil.bin is /evil.bin, outside data_dir. So a malicious/compromised model config can write arbitrary files outside the data directory.

Repro (CPU)

local_data_path guard passes? actually writes to escapes dir?
model.bin yes /data/model.bin no
../evil.bin yes /evil.bin yes
../../etc/passwd yes /private/etc/passwd yes

Fix

Resolve the path before the containment check and compare against the resolved data_dir:

data_dir_resolved = data_dir.resolve()
path = (data_dir / item.local_data_path).resolve()
if path != data_dir_resolved and data_dir_resolved not in path.parents:
    raise ValueError(...)

After the fix, ../evil.bin and ../../etc/passwd are correctly rejected while model.bin, a/b/model.bin, ./x.bin still pass. ruff check / ruff format clean.

@CLAassistant

CLAassistant commented Jun 5, 2026

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@cretz

cretz commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Thanks for the contribution! Can you sign CLA (link above). Also, can you clarify how you came about this issue? Specifically, how are you using truss and this utility today?

@lollinng

lollinng commented Jun 5, 2026

Copy link
Copy Markdown
Author

signed

The guard computed path = data_dir / item.local_data_path and rejected it only
if data_dir not in path.parents. pathlib does not normalize "..", so
"../evil.bin" yields "/data/../evil.bin" which still has data_dir in .parents
and passes -- yet the download paths below call .resolve() and write to
"/evil.bin", outside data_dir. Resolve before the containment check.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@lollinng
lollinng force-pushed the fix/external-data-path-traversal branch from 70ef985 to 5a25515 Compare June 5, 2026 20:55
@cretz

cretz commented Jun 8, 2026

Copy link
Copy Markdown
Contributor

Can you clarify how you came about this issue? Specifically, how are you using truss and this utility today?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants