fyi @SomeRandomDeveloper

Patch for review: https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1183708

Should this not have been done through a security patch and not on gerrit?

Please do NOT upload security patches to gerrit. Please overwrite it with a different commit and abandon it, thanks

I think the issue this fixes was only introduced in fb856ce9cf121e046305116852cca4899ecb48ca, so it does not exist on a release version.

However, the issue is unpatched in WMF production so we should still use the security patch process to fix the issue first AFAICS.

I was told by @sbassett in T360407#9654079 that i18n xss patches are low-risk enough to publish through Gerrit.

In T403411#11137040, @Dreamy_Jazz wrote:

I think the issue this fixes was only introduced in fb856ce9cf121e046305116852cca4899ecb48ca, so it does not exist on a release version.

I can reproduce it on my localhost instance on master, so I don't think it's fixed yet.

In T403411#11137041, @jhsoby wrote:

I was told by @sbassett in T360407#9654079 that i18n xss patches are low-risk enough to publish through Gerrit.

Perhaps, though that example you give was for an extension (ReadingLists) which isn't bundled.

That may have been the case a year ago, but it is not anymore. Especially not for MW core

In T403411#11137046, @jhsoby wrote:

In T403411#11137040, @Dreamy_Jazz wrote:

I think the issue this fixes was only introduced in fb856ce9cf121e046305116852cca4899ecb48ca, so it does not exist on a release version.

I can reproduce it on my localhost instance on master, so I don't think it's fixed yet.

It was introduced in that version, not fixed.

In T403411#11137081, @SomeRandomDeveloper wrote:

It was introduced in thar version, not fixed.

Ah, right, I misread you – sorry.

In T403411#11137049, @SomeRandomDeveloper wrote:

That may have been the case a year ago, but it is not anymore. Especially not for MW core

Alright, did what you suggested. Attaching patch here instead.

In T403411#11137041, @jhsoby wrote:

I was told by @sbassett in T360407#9654079 that i18n xss patches are low-risk enough to publish through Gerrit.

I think most of these message-related XSS patches are generally low-risk enough to go through gerrit, if they get posted there. They are still very difficult to exploit since you'd need to compromise an int/ifadmin account or figure out how to successfully bypass our code-review processes for TWN and MediaWiki. That being said, it's probably a best practice to post a proposed security patch on the task, to keep it private, and then we can make the decision on a case-by-case basis. This could probably be better documented on https://wikitech.wikimedia.org/wiki/How_to_deploy_code#Security_patches.

In T403411#11137049, @SomeRandomDeveloper wrote:

That may have been the case a year ago, but it is not anymore. Especially not for MW core

Alright, did what you suggested. Attaching patch here instead.

T403411.patch1 KBDownload

The code in the patch itself looks fine, but can we create a proper security patch? e.g. have a subject line beginning with SECURITY:, have a standard commit message and Bug: reference, etc. See also: https://wikitech.wikimedia.org/wiki/How_to_deploy_code#Creating_a_Security_Patch

Thanks for the input! New patch file created the right way included:

In T403411#11143170, @jhsoby wrote:

Thanks for the input! New patch file created the right way included:

T403411.patch1 KBDownload

LGTM, CR+2. Thanks.

In T403411#11141589, @sbassett wrote:

This could probably be better documented on https://wikitech.wikimedia.org/wiki/How_to_deploy_code#Security_patches.

I've made a (hopefully clarifying) edit to wikitech here: https://wikitech.wikimedia.org/w/index.php?title=How_to_deploy_code&diff=2338910&oldid=2331871

The above patch was deployed today to wmf.17: https://sal.toolforge.org/log/uHuhFpkBvg159pQrAZsF. Seems stable so far.

Change #1193224 had a related patch set uploaded (by Reedy; author: Jon Harald Søby):

[mediawiki/core@master] SECURITY: Fix XSS vulnerability in Special:Watchlist

https://gerrit.wikimedia.org/r/1193224

Change #1193224 merged by jenkins-bot:

[mediawiki/core@master] SECURITY: Fix XSS vulnerability in Special:Watchlist

https://gerrit.wikimedia.org/r/1193224