HostedRedmine.com has moved to the Planio platform. All logins and passwords remained the same. All users will be able to login and use Redmine just as before. Read more...
Bug #857342
closedQt: more care over HTML-escaping
0%
Description
The Qt client frequently takes strings from the ruleset, city names, usernames etc, and interpolates them into HTML contexts without escaping them.
I've audited for cases where strings were used in the presence of obvious HTML tags in the code. I have probably missed some cases where the code doesn't add tags, but the Qt::AutoText rules ("mainly checks whether there is something that looks like a tag before the first line break") would allow variable text to trigger HTML rendering. I've tried to take a little more care over player-controlled strings (player names, city names) but I have probably missed some of those too. (The most restrictive city name vetting in is_allowed_city_name
doesn't let through any of <, >, or ".)
Tooltips are a bit of a crapshoot, because of their Qt::AutoText behaviour which is not easy to change. I've erred on the side of not escaping, which lets "<i>Silly</i>" through; the alternative could mangle "Fine & Sensible" into "Fine & Sensible". If we wanted to fix this properly, I think we'd have to make our own tooltip override and use it everywhere.
I've not looked at the ruleset editor or modpack installer.
The forthcoming patch is definitely not a complete solution, but it improves things a lot.
This could obviously be argued to be a security risk. I don't see any obvious horrors like <script>
in the Qt HTML subset, but this bug does expose the Qt HTML interpreter/renderer as an attack surface; I don't know if there are other opportunities for mischief (DoS etc).
(I think it would be nice if there were a type transition between plain text and HTML -- the coder currently has to keep track by hand of whether a QString
is supposed to contain text or HTML -- but if we want that, we'd have to invent it ourselves.)
(This was epic yak-shaving on the way to #857341.)
Files
Updated by Jacob Nevins over 3 years ago
- File 26-beastly.patch 26-beastly.patch added
- File 26_alien_html.sav.bz2 26_alien_html.sav.bz2 added
Updated by Jacob Nevins over 3 years ago
- File m-qt-html-escape.patch m-qt-html-escape.patch added
- File 30-qt-html-escape.patch 30-qt-html-escape.patch added
- File 26-qt-html-escape.patch 26-qt-html-escape.patch added
- Status changed from In Progress to Resolved