Skip to content

Fix path traversal in KMS remove_cache (#558)#601

Open
kvinwang wants to merge 1 commit intomasterfrom
fix-issue-558-kms-remove-cache
Open

Fix path traversal in KMS remove_cache (#558)#601
kvinwang wants to merge 1 commit intomasterfrom
fix-issue-558-kms-remove-cache

Conversation

@kvinwang
Copy link
Collaborator

This PR hardens the KMS admin remove_cache endpoint against path traversal.

What changed

  • Treat the sub_dir argument as a relative path and reject absolute paths.
  • Reject any cache keys containing .., root, or prefix components.
  • Normalize the remaining path components and join against the configured cache directory.
  • Use canonical paths (with a fallback when the target does not yet exist) and verify that the resolved path stays within the expected cache directory before deleting anything.
  • Keep the existing sub_dir == "all" behavior for deleting the entire cache directory.

Why
Previously, remove_cache joined sub_dir directly onto the parent cache directory and passed the resulting path into fs::remove_dir_all / fs::remove_file. A caller that knows the admin token could supply a value like ../../etc and cause deletion of directories outside the intended cache root.

The new logic ensures that only normalized, non-escaping paths rooted under the configured cache directory are ever removed.

Testing

  • cargo fmt
  • cargo clippy -p dstack-kms --all-targets -- -D warnings
  • cargo test -p dstack-kms --tests

Closes #558.

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.

Path traversal in KMS remove_cache

1 participant