We need to resolve the issues discovered in T148133
Overall, this looks good. By and large the code seems safe, and most of the issues are either low severity, or very difficult to trigger.
There are two issues I saw come up multiple times. Both are relatively minor as far as security issues go, but still should be fixed:
- Inconsistent encodeURIComponent-ing of url parts in js. Even if the data is from what's probably a safe source, encodeURIComponent should be used to catch special characters, and so we can tell at a glance that it is safe. This is especially true when using JSONP, where there is even the remotest possibility of overriding the callback parameter.
- Using target=_blank for external links without specifying rel=noopener The full list of issues are as follows:
- All target=_blank links should have rel=noopener.
- Line 93 gf-preview.js - The find and replace of </head> could lead to XSS. Consider if someone could add html like <div title="</head><img src=x onerror=alert(1)"> to the page. This is perfectly valid and safe prior to the regex (albeit MW usually doesn't allow that sort of thing, but extensions might), but unsafe after the regex. (In practice this would be very hard to pull off as you would have to be able to insert the tag prior to the actual <head/>, but that's not something we should rely on)
- line 46 of gf-preview.js, encodeURIComponent missing from jsonp queries.
- It would be nice if trusted data from other servers would be escaped (langauge_pairs, language_to_domain_mappings) or failing that, checked if they contain an apostrophe and throw an exception.
- Since you're already loading mw.RegExp.escape, the writeCookie() function in web/static/cookies.js should probably use it. (This is very minor)
- line 70 of gf-input.js - Please encodeURIComponent s and t.
- line 63-ish of gf-preview.js - encodeURIComponent of all the parts of the url.
- ditto gf-create.js
- wm-typeahead.js doesn't seem to escape titles properly when doing syntax highlighting. (Perhaps this is more something I should bring up with the portal people)
- You may want to consider using CORS instead of JSONP for api communication. The mediawiki api supports anonymous CORS if you provide an &origin=* url parameter. From what I understand this is more reliable than jsonp, and also less scary as it doesn't rely on script execution.