Skip to content

fix(transformers): validate archive extraction paths#28777

Merged
tianleiwu merged 2 commits into
mainfrom
tlwu/20260603/icm_6
Jun 4, 2026
Merged

fix(transformers): validate archive extraction paths#28777
tianleiwu merged 2 commits into
mainfrom
tlwu/20260603/icm_6

Conversation

@tianleiwu

Copy link
Copy Markdown
Contributor

Summary

This PR hardens TensorFlow checkpoint archive extraction in the transformer conversion utility by validating archive member paths before extraction and using Python's safe tar extraction filter when available. It also adds regression tests to confirm traversal-style archive members are rejected for both tar.gz and zip inputs.

Key Changes

  • Added a shared safe extraction path for tar.gz and zip checkpoint archives in onnxruntime/python/tools/transformers/convert_tf_models_to_pytorch.py.
  • Replaced direct extractall() usage with path validation to prevent directory traversal outside the target checkpoint directory.
  • Added regression tests in test_convert_tf_models_to_pytorch.py to cover malicious ../ archive members.

Testing

  • ./.venv/bin/python -m pytest test_convert_tf_models_to_pytorch.py

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Hardens the TensorFlow checkpoint download/conversion utility by adding a shared “safe archive extraction” helper to prevent directory traversal when unpacking .tar.gz and .zip checkpoint archives, and adds regression coverage for traversal-style archive members.

Changes:

  • Added _is_safe_archive_member + safe_extract_archive() and routed checkpoint extraction through it.
  • Uses Python tarfile extraction filters (filter="data") when available.
  • Added regression tests to ensure ../ archive members are rejected for both tar.gz and zip.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
onnxruntime/python/tools/transformers/convert_tf_models_to_pytorch.py Introduces path validation and a unified safe extraction helper for tar/zip archives used in checkpoint download.
test_convert_tf_models_to_pytorch.py Adds regression tests for rejecting traversal entries in tar.gz/zip archives.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread onnxruntime/python/tools/transformers/convert_tf_models_to_pytorch.py Outdated
Comment thread onnxruntime/python/tools/transformers/convert_tf_models_to_pytorch.py Outdated
Comment thread test_convert_tf_models_to_pytorch.py Outdated
- _is_safe_archive_member: normalize separators and treat commonpath
  ValueError (mixed-drive paths) as unsafe
- refuse tar extraction when the data filter is unavailable; reject
  symlink/hardlink members
- move regression test under onnxruntime/test/python/transformers/ and
  resolve module path via __file__
- add backslash-traversal and symlink rejection tests

@skottmckay skottmckay left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bit dubious that it was worth us wasting our time on this issue. Seems extremely low priority/Won't Fix given it's a script a user would have to manually run.

@tianleiwu
tianleiwu enabled auto-merge (squash) June 4, 2026 15:03
@tianleiwu
tianleiwu merged commit f1f0436 into main Jun 4, 2026
87 of 89 checks passed
@tianleiwu
tianleiwu deleted the tlwu/20260603/icm_6 branch June 4, 2026 15:55
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.

4 participants