Backport 0.14.1 #2389
Merged
Backport 0.14.1 #2389
Conversation
This file contains 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
The SDK joins paths in two cases: == download_reply == A malicious server could override the filename given in the content-disposition header to trigger an arbitrary file write into e.g. ~/.config/autostart. The fix is to do what download_submission does, and use the pre-sanitized filename. We shouldn't even use the content-disposition header at all, so it's removed from StreamedResponse. == download_submission == In this case no path traversal was possible, since we correctly use submission.filename, which is already validated when it is first fetched from the server. However, we can apply an extra level of hardening by checking for path traversal before we write anything. Fixes <freedomofpress/securedrop-security#108>. (cherry picked from commit 45d86bc4cdcbe6e625b27ac0c15d24f42fbd6c47) (cherry picked from commit 120bac1)
Prevent directory traversal attacks by no longer relying on compromized-vm-provided name. This parameter should instead be obtained by the environment variable $QREXEC_REMOTE_DOMAIN. (cherry picked from commit 9f1580cdde24365084aafffedf367c32d867df8b) (cherry picked from commit 3012bf7)
Validation is no longer necessary, since QREXEC_REMOTE_DOMAIN is provided by dom0 (as opposed to an attacker) and the VM name already includes validations which prevent "/" in the name. (cherry picked from commit 48382568798070c484cbcb82637f8bb42300e976) (cherry picked from commit a4d28c0)
(cherry picked from commit ec5ca97e45e911e3ff867c01fedc94c16bd487f3) (cherry picked from commit 7ab0ad8)
This is a compromise: We have to at least consult the server-provided filename no matter what, because that's how we reconcile our local ordering with the server's authoritative ordering within the source conversation. At that point, we might as well just use it, subject to the same validation enforced for metadata syncs in storage.sanitize_submissions_or_replies(). (cherry picked from commit 6014eece28f04092d7bcf32ebd620e9f394f07f9) (cherry picked from commit 0d521f5)
(cherry picked from commit 884ee7fe6a1c6d94bc203576ce68bdd434d2aafa) (cherry picked from commit e1c6e75)
(cherry picked from commit b3ced68)
(cherry picked from commit 564be9e)
rocodes
approved these changes
Feb 13, 2025
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.
Status
Ready for review
Description
Cherry-pick commits from 0.14.1 release to main. Note that some commits have multiple cherry-pick tags because they were first cherry-picked to the release branch and then back to main 🙃 .
One extra commit to bump the date for the rc changelog entry to make lintian happy.
Test Plan