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

Improperly escaped file names in WebUI #11209

Open
xnoreq opened this issue Sep 8, 2019 · 5 comments · May be fixed by #11215

Comments

@xnoreq
Copy link

commented Sep 8, 2019

Please provide the following information

qBittorrent version and Operating System

v4.2.0alpha1, linux

What is the problem

In the WebUI, content tab, a torrent with a file named "a&b.txt" is displayed as
a&b.txt

What is the expected behavior

Files should have the same name as in the torrent file, desktop application.

Steps to reproduce

Load attached torrent (sorry, had to gz to be able to attach it) into WebUI, go to contents tab.
a&b.txt.torrent.gz

@thalieht thalieht added the WebUI label Sep 8, 2019

@Chocobo1

This comment has been minimized.

Copy link
Member

commented Sep 9, 2019

IIRC, it was done to avoid injection attacks. I wish there is a better way, but so far I'm not aware of.

@xnoreq

This comment has been minimized.

Copy link
Author

commented Sep 9, 2019

The problem is that currently the strings are escaped at least twice:

const name = escapeHtml(file.name);

$(fileNameId).textContent = escapeHtml(value);

@xnoreq

This comment has been minimized.

Copy link
Author

commented Sep 9, 2019

It's 3 times:

  1. prop-files.js makes the request, escapes the name,
  2. dynamicTable.js escapes the file name again,
  3. mootools escapes the resulting value again because the text property is set (and not the (inner)html property) [1] [2]
@Chocobo1

This comment has been minimized.

Copy link
Member

commented Sep 9, 2019

@Piccirello
Looping you in, hope you don't mind.

@xnoreq
Seems you tracked it down already, you might consider contribute a PR as well.

@xnoreq

This comment has been minimized.

Copy link
Author

commented Sep 10, 2019

Sure, I can give it a try.

@xnoreq xnoreq referenced a pull request that will close this issue Sep 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.