Qt: more care over HTML-escaping
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.)