This repository has been archived by the owner on Jan 5, 2024. It is now read-only.
Check extract path and ensure minimal perms #67
Merged
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.
Summary
When a file is received from
sd-app
, ensure that directories are created using minimal permissions and extracted only to allowed paths.Test Plan for Export
Sanity check
..
and/
)/
gets stripped and you can't attach a file that leads with a..
Ensure safe extraction
make semgrep
rule:semgrep.makedirs-insecure-permissions
failrule:semgrep.tarfile-extractall-traversal
failmake test
test_extract_tarball
fail because files have insecure 644 permissions instead of 600test_extract_tarball_raises_if_doing_path_traversal
fail because it does not raise (also see that/tmp/traversed
exists)test_extract_tarball_raises_if_doing_path_traversal_with_symlink
fail because it does not raiserm /tmp/traversed
and checkout second commit in this PR (with the fix)make semgrep
make test
test_extract_tarball_raises_if_doing_path_traversal
andtest_extract_tarball_raises_if_doing_path_traversal_with_symlink
, andqvm-open-in-vm
fromsd-app
tosd-devices
the way you see in the audit to perform this check another way.Ensure no regressions