Skip to content

fix: reject checkpoint filenames that leave the model directory - #226

Open
shiaho777 wants to merge 1 commit into
modelscope:mainfrom
shiaho777:fix/checkpoint-path-escape
Open

shiaho777 wants to merge 1 commit into
modelscope:mainfrom
shiaho777:fix/checkpoint-path-escape

Conversation

@shiaho777

Copy link
Copy Markdown
Contributor

SafetensorLazyLoader joined the model directory with the filename from model.safetensors.index.json and opened the result. os.path.join drops the directory when the filename is absolute, and a relative name such as ../../other.safetensors resolves outside the directory.

checkpoint_path realpaths both sides and requires the file to stay under the model directory. Absolute names are rejected before the join. The index file itself is still the fixed name model.safetensors.index.json.

Checked with python3 -m pytest tests/test_checkpoint_path.py. A shard inside the directory resolves, and both ../outside.safetensors and an absolute path raise ValueError. flake8 is clean.

SafetensorLazyLoader joined the model directory with the filename from model.safetensors.index.json and opened the result. os.path.join drops the directory when the filename is absolute, and a relative name such as ../../other.safetensors resolves outside the directory.

checkpoint_path realpaths both sides and requires the file to stay under the model directory. Absolute names are rejected before the join. The index file itself is still the fixed name model.safetensors.index.json.

Checked with python3 -m pytest tests/test_checkpoint_path.py. A shard inside the directory resolves, and both ../outside.safetensors and an absolute path raise ValueError.
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.

1 participant