Repository navigation
Refuse a download path that climbs out of /user_uploads - #380
Merged
Merged
Conversation
An upload path was accepted on its /user_uploads/ prefix alone. The HTTP client resolves dot segments, their percent-encoded forms and backslashes before it sends, so a path such as /user_uploads/../api/v1/users reached an API endpoint with the minter's credentials. The upload-path brand now accepts only a path that URL parsing leaves unchanged once decoded, and whose segments decode to no separator. Both the adapter's downloadFile and the download_file tool decode through it, so a refused path sends no request. The shared attachment contract now tries those climbs, plainly written and percent-encoded, and checks that a file with a non-ASCII name still downloads. To make that second test mean what it does on Zulip, the in-process realm now sanitises upload names the way Zulip does, keeping Unicode word characters, and decodes a request path before it routes it.
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.
An upload path was accepted on its
/user_uploads/prefix alone. The HTTP client resolves dot segments, their percent-encoded forms and backslashes before it sends, so a path such as/user_uploads/../api/v1/usersreached an API endpoint with the minter's credentials. Any agent could pass such a path throughdownload_file.The upload-path brand now accepts only a path that URL parsing leaves unchanged once decoded, and whose segments decode to no separator. The adapter and the tool both decode through the brand, so a refused path sends no request.
The shared attachment contract now tries those climbs and checks that a non-ASCII filename still downloads. For the second test to match Zulip, the in-process realm now keeps Unicode word characters in upload names as Zulip's
sanitize_namedoes, and decodes a request path before routing it.