Limit what code points are allowed in suffixes. #252
Merged
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
Especially in save dialogs there is danger in allowing arbitrary code points in file extensions/suffixes. Certain suffixes have special meaning or behavior on certain platforms. For predictibality purposes we enforce the same restrictions on all platforms. Limiting extensions to [A-Za-z0-9!#$+-._] still allows all common (and less common) extensions, so shouldn't be needlessly limiting.
…ted. After more discussion a more restricted character set seems just as good. Also leave a bit more up to user agent discretion when it comes to blocking file types such as .lnk and .local.
pwnall
approved these changes
Dec 2, 2020
1 task
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.
There are a number of issues with what we allow in extensions that a website can provide when showing a file picker. Since the file picker (on most platforms) appends these extensions to the filename the user enters, this can result in filenames with characters we don’t want to allow/that are otherwise problematic. In particular we don't want to allow control characters or whitespace in suffixes, or filenames that end in a '.'. As such this PR adds restrictions on what characters are allowed in accepts file extensions/suffixes, as well as limiting their length to 16.
Limiting extensions to only contain alphanumeric characters, + or . still allows all
extensions in the shared-mime-info database as well as nearly all extensions in Wikipedia's List of filename extensions.
On Windows file suffixes such as
.lnkand.localare also particularly dangerous to be written to. We opted not to reject these when specified as "accepted" file types inshowSaveFilePickerthough, instead leaving it up to the user agent to ignore these file suffixes, and/or block writing to these file types completely.Considered alternatives
Have a bigger allow list of characters. Other characters, such as +, -, $, # and ! are probably safe in file extensions, and have been used occasionally by applications in the past. Because of how rarely these are used, we're limiting extensions to just those characters that are at least somewhat in common use.
Have a block list rather than an allow list. This option was rejected since while we could try to enumerate all characters that might be problematic, the consequences of not blocking something we should have blocked are much worse than not allowing something we could have allowed. As such a minimal allow list was chosen instead.
Also reject when suffixes ending in
.lnkor.localappear in the accepted file types for a showSaveFilePicker or showOpenFilePicker call. We opted against this because different platforms will likely have different extensions they might want to block. Rejecting for these only on some platforms would result in unpredictability where a website might work fine on one platform but fail on another. Rejecting these on platforms where these suffixes don't have special meaning would be needlessly limiting. Giving user agents the option to ignore these file suffixes, or at least not auto-append them to file names in save dialogs still protects the user without these downsides.Preview | Diff