sanitize volume and atom names in webc_to_package_dir - #6711
Open
netliomax25-code wants to merge 2 commits into
Open
sanitize volume and atom names in webc_to_package_dir#6711netliomax25-code wants to merge 2 commits into
netliomax25-code wants to merge 2 commits into
Conversation
theduke
requested changes
Jun 16, 2026
theduke
left a comment
theduke
left a comment
Collaborator
There was a problem hiding this comment.
This isn't explicitly documented , but we would like unpack to be "idempotent", meaning you can run unpack and build and get the same package back.
This is a valid concern though.
We could add a --allow-escape flag or similar.
If not provided , escaping the main package directory would lead to an error, with a hint to use the flag.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens webc_to_package_dir against path traversal by sanitizing container-controlled names before constructing on-disk extraction paths, preventing a hostile WebC from unpacking files outside the requested output directory.
Changes:
- Sanitize filesystem volume names before joining them onto
target_dirduring volume unpack. - Sanitize atom names before joining them onto
module_dirwhen writing atom files. - Add a regression test covering a volume named
../escaped.
Comment on lines
147
to
151
| // Atom names also come from the container, so sanitize before | ||
| // joining to keep the write inside module_dir. | ||
| let atom_subpath = webc::sanitize_path(&atom_name); | ||
| let atom_path = module_dir.join(atom_subpath.trim_start_matches('/')); | ||
|
|
Comment on lines
93
to
99
| // The volume name is taken verbatim from the (untrusted) webc and | ||
| // is used to build an on-disk path. Without normalization a name | ||
| // such as "../x" would unpack outside target_dir, so route it | ||
| // through the same sanitizer used elsewhere in the crate. | ||
| let volume_subpath = webc::sanitize_path(&mapping.volume_name); | ||
| let volume_path = target_dir.join(volume_subpath.trim_start_matches('/')); | ||
|
|
Comment on lines
+296
to
+305
| webc_to_package_dir(&container, &target_dir).unwrap(); | ||
|
|
||
| // The volume contents must stay under target_dir, never in a sibling. | ||
| let escaped = root.join("escaped").join("secret"); | ||
| assert!( | ||
| !escaped.exists(), | ||
| "volume escaped the output directory to {}", | ||
| escaped.display() | ||
| ); | ||
| assert!(target_dir.join("escaped").join("secret").exists()); |
Contributor
Author
|
Good point on idempotency. Reworked it along the lines you suggested:
This way nothing is rewritten, so unpack/build stays consistent. |
Contributor
Author
|
any update? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
webc_to_package_dirbuilds the on-disk destination for each volume astarget_dir.join(mapping.volume_name.trim_start_matches('/')), and for each atom asmodule_dir.join(atom_name). Both names come straight from the (untrusted) webc, andtrim_start_matches('/')only removes a leading slash, not...../escapedthen makesvolume.unpackwrite under the parent oftarget_dir, sowasmer package unpackof a hostile package can drop files outside the chosen output directory.--allow-escapeflag is not set, the conversion errors out with a hint to use the flag; with the flag set the original behavior is preserved.Added a regression test that unpacks a webc whose volume is named
../escapedand checks that the default rejects it (writing nothing to a sibling directory) whileallow_escapelets it through.