Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HTML-encode file names in AjaxFileUpload #483

Merged
merged 2 commits into from May 8, 2019

Conversation

@AlekseyMartynov
Copy link
Member

commented May 7, 2019

On Linux and Mac, it's possible to use unsafe HTML characters in file names. File names are not encoded in the AjaxFileUpload.js script.

Technically, it's an XSS vulnerability. However, the attack vector is very limited. Pre-conditions:

  • An end-user initiates the upload of a file with malicious code contained within its name.
  • Non-Windows server (e.g. Mono on Linux).
  • ASP.NET request validation is disabled for the request.
  • A temp file name generated by AjaxFileUpload is displayed without encoding to another user.

On Window servers, malicious file names always cause a server-side error thanks to invalid characters checks:

Therefore, malicious code can only be executed in the same browser session in which the file is uploaded.

This PR:

  • Adds file name HTML encoding on the client.
  • Explicitly adds unsafe HTML chars to the invalid list, in addition to Path.GetInvalidFileNameChars.

@alexxanderz
Copy link
Contributor

left a comment

Do we also need to replace the single ' quote character as well here?

@AlekseyMartynov

This comment has been minimized.

Copy link
Member Author

commented May 7, 2019

@alexxanderz

There's a discussion about escaping single quote: https://webmasters.stackexchange.com/q/12335. Sometimes it's important to HTML-encode it.

My motivation:

  • I'm introducing Sys.Extended.UI.htmlEncode as a generic helper that can be used in other places. It's better to over-encode in it.
  • For a file name, single quote is fine (What's New.txt). That's why I skipped it.

@alexxanderz alexxanderz merged commit 56e7115 into DevExpress:master May 8, 2019

1 check passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@AlekseyMartynov AlekseyMartynov deleted the AlekseyMartynov:html-encode branch May 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.