Repository navigation
Escape file names and paths in directory listing HTML - #2005
Open
anshal-lang wants to merge 1 commit into
Open
anshal-lang wants to merge 1 commit into
anshal-lang wants to merge 1 commit into
Conversation
DirectoryUtils wrote resource names, the parent link and the request path into single-quoted href attributes and element text without escaping, so a file named x' onmouseover='alert(1).txt broke out of the attribute and injected script into the generated listing page. Signed-off-by: anshal ossaimi <anshal@medios.digiscrypt.com>
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.
The directory listing produced by
DirectoryUtils.renderDirectoryListing(used byResourceHandlerandDefaultServletwhen listing is enabled) writes each resource name, the parent link and the request path straight into single-quotedhrefattributes and element text. None of those values are HTML-escaped, and on most filesystems a file name can legally contain',<,>and&. A file namedx' onmouseover='alert(1).txtplaced in a listed directory, for example through an upload feature that writes into it, renders as<a class='icon file' href='/path/d'q/x' onmouseover='alert(1)&.txt'>, so script runs in the origin of anyone who browses the listing. I found it while reading the listing code and confirmed it with the newFileHandlerTestCase.testDirectoryListingEscapesNames, which fails on main and passes with this change. The fix escapes&,<,>,"and'insideDirectoryUtilsat the point the markup is built, which keeps link targets the same for browsers because entity references in attributes decode back to the original characters.