Escape mod name in mod pack typeahead

[DasSkelett]

Problem

While we worked on #384, I also created a mod with a name that contains valid HTML with valid JavaScript.
I was relieved that I didn't find any place where this JavaScript is executed, it looked like we / the browser correctly escapes it everywhere (it does so inside header tags like <h1> for example).

Well, now I was looking at #380 and trying it out, and it turns out that the typeahead library does not escape HTML in its preview drop down:

You could argue this is a bug in the library, and they did actually fix it in release 0.11.0 (we're at 0.10.2):

• XSS vulnerability by default twitter/typeahead.js#964
• Escape html chars from display value when using default template twitter/typeahead.js#1182

But at the same time I'd argue that our API (or at least the frontend that consumes it) should do sanitization as well. After asking in the Discord, VITAS responded that it should be done in the API, HebaruSan favors the frontend.

Changes

Now we run the mod name through a escape_html() function, as suggested here on SO, before passing it to the typeahead library to display it.

We might also want to update the library at some point, however I spotted breaking changes while looking through the docs, and this library is also unmaintained got 6 years now anyway, so maybe we should switch to something else entirely.

 

Also the background image link in the mod API response is fixed, after #374 this would've caused the file paths instead of web URLs to be returned.

[HebaruSan]

After asking in the Discord, VITAS responded that it should be done in the API.

I don't think I agree. Escaping like this can only be done correctly if we know the context where the output will be used, and we don't. The current code assumes an HTML context, but for example the CKAN bot will consume this data and almost never put it into an HTML context.

Example of when this can go wrong:
Suppose my short_description contains "Compatible with KSP <= 1.10". escape would mangle this to "Compatible with KSP &lt;= 1.10" (pardon double-escaping to get past Markdown), which would then feed into CKAN's metadata, where it would be unreadable to users unless they know how HTML escape sequences work. CKAN and other clients would have to unescape the text to get back to something usable.

Raw / unescaped output makes sense for the API (except for those minimal changes needed to stuff it into JSON, of course). Consumers should apply their own processing as needed.

[DasSkelett]

Then let's do that instead. Would've been nice because it would've caught all places of possible XSS through data received from the API in one go, now I can only close the one I know about.
You gonna tell VITAS ^^

Btw., there is one place where CKAN's infrastructure blindly takes whatever SpaceDock serves it (although not through the API, but as webhook POST body), and places it into an HTML context: the SpaceDock adder
Fortunately GitHub does its own sanitization.

[HebaruSan]

You gonna tell VITAS ^^

Is there any way we can approach this as him having to convince me instead? 😏

[HebaruSan]

Makes sense, let's go.

[DasSkelett]

You can work it out between the two of you. I just want this huge security issue fixed.