Patch:

In T395949#10881239, @SomeRandomDeveloper wrote:

Patch:

01-T395949.patch2 KBDownload

(edit: updated the commit message)

Patch looks good to me. This should just go through gerrit to get merged. Though it doesn't look like this extension has been actively maintained for many years.

@SomeRandomDeveloper: Very nice find and detailed write-up and functional patch, thanks for all this! 😍 Patch is fine to land as-is, but mandatory nitpicks:

• might be worthwhile to bump the extension's version number in extension.json
• likewise, I'd suggest making the minimum required MW version 1.43 as (per mw:Version lifecycle) it's the current LTS version and the namespaced Html class is available there; otherwise I'd suggest 1.39, which is the legacy LTS version, but the namespaced Html class is not available there; either way, 1.40.x series reached their EOL about a year ago
• if going with the 1.43 suggestion, might add a use MediaWiki\Parser\Sanitizer; statement to the top of the file as well as the Sanitizer class is namespaced in MW 1.43(+)
• I'd suggest decreasing indentation on patch lines 71-76 by one

Again, these are more cosmetic suggestions rather than anything else, as the patch works just fine and fixes the reported vulnerability. 👍

@sbassett: Different extensions (and other components for that matter) vary in complexity. GoogleDocs4MW is pretty much about as basic of a parser hook tag extension as one can be; originally it was just a single PHP file in its initial public release. Nowadays you "need" two files (the main PHP part and extension.json) to be able to use wfLoadExtension() in LocalSettings.php to load the extension, but everything else in the repo (i18n, CI, etc.) is not strictly necessary.

So no, it's not always as simple as "more updates = better code" and "hasn't been touched in X years = ready to be archived".

It's really more about finding maintainers than about the actual usage of an extension or skin. The current Developers/Maintainers page is pretty laughably out-of-date and flawed in other ways. And I don't see an entry for GoogleDocs4MW. So when the Security-Team triages all reported security issues each week, we need to try to find maintainers via git history, people with domain knowledge of the code and other imperfect methods. When we look at a git history and only see minor, automated commits for several years, that often doesn't bode well in finding anyone willing to review and merge security patches. When it comes to actual usage stats for a given extension or skin, we also do not have great data for that either, IMO - wikiapiary.com isn't all that reliable in my experience. So we generally just leave repos alone and only attempt to archive them if there's a lot of convincing evidence that the extension or skin is no longer used or has been migrated to a newer extension, into core, replaced by a gadget, etc.

In T395949#10886263, @ashley wrote:

@SomeRandomDeveloper: Very nice find and detailed write-up and functional patch, thanks for all this! 😍 Patch is fine to land as-is, but mandatory nitpicks:

• might be worthwhile to bump the extension's version number in extension.json
• likewise, I'd suggest making the minimum required MW version 1.43 as (per mw:Version lifecycle) it's the current LTS version and the namespaced Html class is available there; otherwise I'd suggest 1.39, which is the legacy LTS version, but the namespaced Html class is not available there; either way, 1.40.x series reached their EOL about a year ago
• if going with the 1.43 suggestion, might add a use MediaWiki\Parser\Sanitizer; statement to the top of the file as well as the Sanitizer class is namespaced in MW 1.43(+)
• I'd suggest decreasing indentation on patch lines 71-76 by one

Again, these are more cosmetic suggestions rather than anything else, as the patch works just fine and fixes the reported vulnerability. 👍

Thanks for the feedback. I will incorporate these changes and provide a new patch shortly.

Updated patch:

In T395949#10887814, @sbassett wrote:

It's really more about finding maintainers than about the actual usage of an extension or skin. The current Developers/Maintainers page is pretty laughably out-of-date and flawed in other ways.

Indeed, but at least it's translated into various non-English languages. ;-)

And I don't see an entry for GoogleDocs4MW.

That's because the page is, as it was originally created to be, very WMF-centric; "by the WMF, for the WMF", if you will. I, for example, maintain a bunch of things, the vast majority which are not WMF-deployed and probably never will; the same goes for people like the Semantic MediaWiki community, Yaron Koren, Hallo! Welt/BlueSpice developers, etc. - in my view, people like us are more "MediaWiki" than "Wikimedia", even if we certainly appreciate the technical infrastructure etc. provided by the Foundation, and though the majority of our creations are not WMF-deployed, the wider MediaWiki community certainly does use and appreciate them.

So when the Security-Team triages all reported security issues each week, we need to try to find maintainers via git history, people with domain knowledge of the code and other imperfect methods. When we look at a git history and only see minor, automated commits for several years, that often doesn't bode well in finding anyone willing to review and merge security patches.

Fair. But surely the thing's primary/named "author(s)", as defined in extension.json/skin.json/potential PHP entry point (if any) would be a decent starting point in addition to git commit data?

When it comes to actual usage stats for a given extension or skin, we also do not have great data for that either, IMO - wikiapiary.com isn't all that reliable in my experience.

Alas, that is the current state of things. :-/ Its uptime and performance in the recent years has been less than stellar. There would definitely be a need to have a wider discussion about that, for I think such a resource, if and when it'd work as intended, would be greatly beneficial to all parties involved - developers and end-users alike.

In T395949#10888219, @SomeRandomDeveloper wrote:

Updated patch:

02-T395949.patch2 KBDownload

LGTM, please feel free to submit to gerrit whenever and I'll +2 it unless someone else beats me to it! 👍

Change #1155269 had a related patch set uploaded (by SomeRandomDeveloper; author: SomeRandomDeveloper):

[mediawiki/extensions/GoogleDocs4MW@master] SECURITY: Sanitize style parameter properly

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

Change #1155270 had a related patch set uploaded (by SomeRandomDeveloper; author: SomeRandomDeveloper):

[mediawiki/extensions/GoogleDocs4MW@REL1_44] SECURITY: Sanitize style parameter properly

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

Change #1155271 had a related patch set uploaded (by SomeRandomDeveloper; author: SomeRandomDeveloper):

[mediawiki/extensions/GoogleDocs4MW@REL1_43] SECURITY: Sanitize style parameter properly

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

Change #1155272 had a related patch set uploaded (by SomeRandomDeveloper; author: SomeRandomDeveloper):

[mediawiki/extensions/GoogleDocs4MW@REL1_42] SECURITY: Sanitize style parameter properly

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

Change #1155271 merged by jenkins-bot:

[mediawiki/extensions/GoogleDocs4MW@REL1_43] SECURITY: Sanitize style parameter properly

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

Change #1155270 merged by jenkins-bot:

[mediawiki/extensions/GoogleDocs4MW@REL1_44] SECURITY: Sanitize style parameter properly

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

Change #1155272 merged by jenkins-bot:

[mediawiki/extensions/GoogleDocs4MW@REL1_42] SECURITY: Sanitize style parameter properly

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

Change #1155269 merged by jenkins-bot:

[mediawiki/extensions/GoogleDocs4MW@master] SECURITY: Sanitize style parameter properly

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

In T395949#10900626, @SomeRandomDeveloper wrote:

All merged and no longer reproducible.

Thanks. I'll open up this task. And we're now tracking this for the next supplemental security release (T389312).

Change #1178865 had a related patch set uploaded (by Lewis Cawte; author: SomeRandomDeveloper):

[mediawiki/extensions/GoogleDocs4MW@REL1_39] SECURITY: Sanitize style parameter properly

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

Change #1178865 merged by jenkins-bot:

[mediawiki/extensions/GoogleDocs4MW@REL1_39] SECURITY: Sanitize style parameter properly

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