Page MenuHomePhabricator

Loading JS from user space where the username is not a registered account is dangerous and should be banned
Closed, ResolvedPublic

Description

If someone creates a page User:Foo/bar.js and someone else does importScript( 'User:Foo/bar.js' );, but there is no account named User:Foo, this is very dangerous as anyone can create User:Foo and take control of the script.

While it doesn't solve the problem entirely, banning loading such pages with text/javascript mime type (e.g. making http://mywiki.com/w/index.php?title=User:Foo/bar.js&action=raw&ctype=text/javascript return a 403) will discourage users from making this kind of mistake.

Event Timeline

Bawolff created this task.Oct 21 2018, 5:43 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 21 2018, 5:43 PM
Bawolff closed this task as Resolved.Oct 21 2018, 5:45 PM
Bawolff claimed this task.
Bawolff added projects: Security, JavaScript.

Fixed in https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/433094/

Note, the gerrit commit references a different bug. Unfortunately that bug in the course of investigating an issue had some comments added to it containing private data. As a result it can never be made public, so I have summarized the issue on this new bug.

Change 468849 had a related patch set uploaded (by Brian Wolff; owner: Brian Wolff):
[mediawiki/core@REL1_31] SECURITY: Disallow loading JS/CSS/Json subpages from unregistered users and log

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

Change 468850 had a related patch set uploaded (by Brian Wolff; owner: Brian Wolff):
[mediawiki/core@REL1_30] SECURITY: Disallow loading JS/CSS/Json subpages from unregistered users and log

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

Change 468855 had a related patch set uploaded (by Brian Wolff; owner: Brian Wolff):
[mediawiki/core@REL1_27] SECURITY: Disallow loading JS/CSS/Json subpages from unregistered users and log

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

Change 468849 merged by jenkins-bot:
[mediawiki/core@REL1_31] SECURITY: Disallow loading JS/CSS/Json subpages from unregistered users and log

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

Change 468855 merged by jenkins-bot:
[mediawiki/core@REL1_27] SECURITY: Disallow loading JS/CSS/Json subpages from unregistered users and log

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

Change 468850 merged by jenkins-bot:
[mediawiki/core@REL1_30] SECURITY: Disallow loading JS/CSS/Json subpages from unregistered users and log

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

What are the conditions in which someone could create User:Foo/bar.js when there is no account for User:Foo? Wouldn't it be better to also disallow creation of these pages by anyone? Or are there valid use-cases for these pages (I can't think of anything that couldn't be better covered by mechanisms such as the sitewide files, gadgets, etc.)?

What are the conditions in which someone could create User:Foo/bar.js when there is no account for User:Foo? Wouldn't it be better to also disallow creation of these pages by anyone? Or are there valid use-cases for these pages (I can't think of anything that couldn't be better covered by mechanisms such as the sitewide files, gadgets, etc.)?

If someone's account was renamed (or possibly deleted if using an extension that deleted account).

Sometimes people just create shared js files in User:Js/scriptFile.js as a shared place for scripts, which is bad and people shouldn't do it.

(I'm not opposed to refining the changes further, this change is more a starting place than a final place)

Dinoguy1000 added a comment.EditedOct 22 2018, 12:35 AM

I thought renaming accounts normally also renamed all the pages in the account's userspace, but I could see how that might fail or just not happen in some cases. For account renames, I feel like MediaWiki should be reserving the old username to prevent someone coming in and registering it, even if MW itself doesn't support account renames per se, but I think this is a separate issue probably.