I'm finding the following instances of the gadget using this global search query:

https://global-search.toolforge.org/?q=bigSecurityWarning&namespaces=2%2C8&title=%28.*%5C.js%29

I have global interface admin rights now, so I can patch all of the affected wikis. It will take me a little while to get the patches ready, though, and it's too late here for me to do it today.

ENWP version patched, let me know how it looks. Edit: As discussed over email, self-reverted and revdel'd so we can coordinate the patch. The patch was adding an escape function in Import.prototype.toJs that does s.replace( /'/g, "\\u0027" ).replace( /\\/g, "\\u005C" ) on the strings going into the JS (this.wiki and this.page).

Thanks for making the patch. I had a look at it, and I think that the URL needs to be escaped differently, with URL encoding instead of JS string escaping. In fact, there are four different contexts here, that each require different escaping:

1. JavaScript string (for the importScript call
2. JavaScript comment (the backlink)
3. URL path (for the wiki name)
4. URL query parameter (for the page in the URL title parameter)

We could probably get away with not escaping the wiki name if it's not controlled by the user, but we should probably try to escape it properly anyway.

Escaping all of these contexts properly is actually really hard, so I'm going to need to think a bit more about this. I did find a good candidate for the JavaScript string escape function from the js-string-escape NPM package, so maybe we can adapt that.

I've created a patch for the enwiki gadget. @Enterprisey: how does this look to you?
I haven't tested the patch locally yet - I'll do that after work.

I believe the line terminator characters are not allowed to be in MediaWiki page titles, so if it's not too much extra effort I'd suggest rejecting them outright (crashing or alert()'ing would be fine, the installation links would have to be severely broken anyway). Otherwise the patch looks good to me as written; I'd be fine with applying it with or without my suggested change.

Should we wait for a global interface editor?

In T300743#7685215, @Enterprisey wrote:

Should we wait for a global interface editor?

FTR, @MrStradivarius is a global interface editor, so they can apply the change globally. I can too, although I'd prefer someone who looked into the issue more deeply to do the applying.

I've tested the patch, and there is a problem with it - it breaks round tripping between the common.js and the gadget. If a script name contains a single quote like foo'bar.js, when saving it to common.js it will be escaped as foo\'bar.js, which is correct. However, if you then normalise the import for that script, the gadget will then parse it from common.js without decoding it, and then double encode it when it saves the page, so you will get foo\\\'bar.js. This will break the import, as that is a different MediaWiki page title than the original script name.

To fix this, we need to either decode the strings when parsing script names from common.js, or introduce some way to mark script names as already decoded. I think the decoding route is probably easier, so I will work on a new patch that does that today.

In T300743#7677493, @Enterprisey wrote:

I believe the line terminator characters are not allowed to be in MediaWiki page titles, so if it's not too much extra effort I'd suggest rejecting them outright (crashing or alert()'ing would be fine, the installation links would have to be severely broken anyway).

This sounds like a good idea, but is probably best implemented as a separate patch, as it will make the current patch more complicated and is not strictly necessary for security. That would also allow for the validation code to be discussed on-wiki where others can give input.

In T300743#7685596, @Urbanecm wrote:

FTR, @MrStradivarius is a global interface editor, so they can apply the change globally. I can too, although I'd prefer someone who looked into the issue more deeply to do the applying.

I'll roll this out when the new patch is ready. I should be able to do it today, as I don't have anything else planned in particular.

In T300743#7686298, @MrStradivarius wrote:

However, if you then normalise the import for that script, the gadget will then parse it from common.js without decoding it, and then double encode it when it saves the page, so you will get foo\\\'bar.js.

Hm, I think I was wrong about this - it looks like everything before the first double or single quote gets chopped off by the regex when parsing the JavaScript. Which still means a broken import, but not one that will be fixed by decoding. I guess the proper way to do this is by making an abstract syntax tree of the common.js page, but that sounds like too much work for this patch.

Here's the new patch:

I found a problem with T300743-enwiki2.patch - disabling and uninstallation was broken for imports containing escaped characters, as they were searching for the unescaped script name instead of the escaped script name. I've fixed this in T300743-enwiki3.patch below. I'm confident that the patch is working properly now, so if everything looks good I will go ahead and roll it out. @Enterprisey, does this look OK to you?

@MrStradivarius that all looks good to me, thanks for all your work on this

Ok, I have now patched all affected scripts, so I'm closing this task. Thanks everyone for your help.

In T300743#7688584, @MrStradivarius wrote:

Ok, I have now patched all affected scripts, so I'm closing this task. Thanks everyone for your help.

Thanks for quickly taking care of these issues.