Page MenuHomePhabricator

Remove or limit edituserjs and similar rights from users with "higher" access than the editor
Open, MediumPublic

Description

The (somewhat misleadingly named) edituserjs/editusercss/edituserjson permissions allow a user to edit another user's personal JS/CSS/JSON files. There is little practical use for this (it might be used to help users who messed up their personal scripts but just telling them what to change would work just as well) and it's a convenient attack vector. Either it should be removed or limited so that users cannot edit the scripts/styles of a user who has more permission (e.g. an admin should not be able to edit a bureaucrat's personal scripts; a bureaucrat should not be able to edit a steward's).

(This issue has been discussed in various places, but a dedicated task did not exist.)

Event Timeline

@TheDJ has done quite a bit of work maintaining user scripts (I guess he would prefer not to do it, but I do know he currently provides his time in this fashion) that less-experienced users would probably not be willing (read: "sure-enough") or able to do. Removing the permissions is not sensible from this dimension.

I would guess with T190015: Create separate user group for editing sitewide CSS/JavaScript that does not include administrators by default he would still be a member of the technical administrator group. Were we to limit what tech admin can edit, I don't think a reliance on other rights packages makes sense in this context.

@Izno do less-experienced users involve admins, stewards etc? The main concern here is that someone compromising a technical administrator account can use this ability to take over something more powerful, like a steward account. (That could be done by editing non-personal Javascript as well, but that's much easier to notice - there are only so many JS pages that get loaded and a large number of people are bound to be watching them. Creating User:Foo/vector.js is much more likely to avoid notice.)

An option to reduce the attack vector, while allowing other users to "clean" broken code, would be the ability to only blank or delete the page, instead of modifying it. Blanking the page wouldn't compromise anything.

I'd prefer blanking over deleting, because the user can recover or revert it on its own, while deleting the page will remove that possibility except for users with the right to see deleted revisions. However, I don't know how this could be implemented in the UI

I'd prefer blanking over deleting, because the user can recover or revert it on its own, while deleting the page will remove that possibility except for users with the right to see deleted revisions. However, I don't know how this could be implemented in the UI

Not easily; performance checks happen well before you decide what to save. It could be done with a special page, but that seems a lot of hassle for something like this.

Comparing the permissions of the page owner to the permissions of the editor and refusing if they are higher still seems like the easiest approach to me (CentralAuth makes it a bit more complicated but probably still doable).

Do you plan to decide based on the number of user rights/permissions of each user? permissionCount(userA) > permissionCount(userB)

@Izno do less-experienced users involve admins, stewards etc?

As an example, many users not intimately familiar with how Javascript and CSS pages work are (appropriately) afraid of the banner that says "this page can be used to execute malicious code". This set of users does indeed include those holding advanced permissions. So in this context, absolutely. (Hence the reason these rights are being removed from the administrator package.)

There is also the ease of it--TheDJ would have to fork the page in question, make the changes, then ask the user to copy his changes, and then nuke whatever copy he had made, if he could not edit the pages directly. This ease is valuable (for the same reason that it is a dangerous vector).

The main concern here is that someone compromising a technical administrator account can use this ability to take over something more powerful, like a steward account.

I understood this to be the case from the description. However, the tech admin group is already going to have mandatory 2FA or some similar requirement, which significantly reduces the probability of an account compromise (maybe even to negligible if not zero). This is one dimension of your classical risk cube.

As for the other dimension of the risk cube (consequence), I want to avoid elaborating too much on the possible consequences of taking an advanced permission holder's account, but either a) those consequences would be logged for review and subsequent alerting or b) are so massive that everyone will understand what has happened. In both cases, Someone will be contacted to remove the rights from the compromised account.

However, the tech admin group is already going to have mandatory 2FA or some similar requirement, which significantly reduces the probability of an account compromise (maybe even to negligible if not zero).

It is reduced, but definitely not negligible. The recent round of compromises had nothing to do with passwords, for example.

I want to avoid elaborating too much on the possible consequences of taking an advanced permission holder's account, but either a) those consequences would be logged for review and subsequent alerting or b) are so massive that everyone will understand what has happened.

The way a typical attack would go, someone compromises a (tech)admin account on eowiktionary, use that to take over an account that's admin on enwiki, then put viagra ads on the enwiki main page or whatever. The time between the two account compromises is the window for preventing the attack; there is decent chance that someone would notice the attack quickly if it's done via MediaWiki:Common.js, much less if it's done via the user js page of some user whom the denizens of that wiki don't even recognize.

I see your point of how this would obstruct some people's work, though. Maybe there is a better way, e.g. flagging such edits as dangerous and having the owner approve them somehow.

Do you plan to decide based on the number of user rights/permissions of each user? permissionCount(userA) > permissionCount(userB)

Obviously not. Probably have a precedence list of very powerful permissions (steward > bureaucrat > admin); if on some wiki userA has a higher-precedence permission than userB then userB editing the JS of userA (on any wiki) should be considered a dangerous edit.

There is also the ease of it--TheDJ would have to fork the page in question, make the changes, then ask the user to copy his changes, and then nuke whatever copy he had made, if he could not edit the pages directly. This ease is valuable (for the same reason that it is a dangerous vector).

Yeah not doing that ;)
I drag people out of a tough spot, but I spent like 5 minutes doing so most of the time. The more work it is, the less I can justify doing these often 'in-between' things. Besides, quite often these people can't even copy paste the scripts without accidentally introducing syntax errors. Many people just see symbols, not javascript when looking at these pages. Which is again, why in modern days this feature would never have been added of course.

Vvjjkkii renamed this task from Remove or limit edituserjs and similar rights to o4aaaaaaaa.Jul 1 2018, 1:04 AM
Vvjjkkii triaged this task as High priority.
Vvjjkkii updated the task description. (Show Details)
Vvjjkkii removed a subscriber: Aklapper.
IKhitron renamed this task from o4aaaaaaaa to Remove or limit edituserjs and similar rights.Jul 1 2018, 3:29 PM
IKhitron raised the priority of this task from High to Needs Triage.
IKhitron updated the task description. (Show Details)
IKhitron added a subscriber: Aklapper.
IKhitron subscribed.

I do think techadmin right is enough. There should always some users that can control the whole websites - if nobody have edituserjs rights it will be impossible to manage libelous and non-public personal information someone may placed on their JS or CSS page.

I think it will be enough to make impossible to edit personal js/css files of users, who can edit sitewide js/css. So, ordinary admins should can edit personal js/css of ordinary users, but not techadmins (maybe admins also). This will prevent the spread of malicious code throughout the site.

It is rare (though valid) that personal js/css can be edited by others, so I think techadmin right is enough. MediaWiki technically does not have a hierachical system of user groups - admins can block any users, including stewards.

edituserjs/editusercss/edituserjson permissions have been revoked from sysops in rMWdb888bc5ad

With this being removed from sysop, is it actually also being ADDED to techadmin - or will it be orphaned?

With this being removed from sysop, is it actually also being ADDED to techadmin - or will it be orphaned?

According to the linked patch, it is being added to tech admin at this time. I think the jury is out as to the necessity of this ticket at this time...

chasemp triaged this task as Medium priority.Dec 9 2019, 4:56 PM

I don't think how this is useful; there's legitimate (though rare) reason to edit others' JS/CSS page from time to time, e.g. to replace the usage of deprecated JS module.

Xaosflux renamed this task from Remove or limit edituserjs and similar rights to Remove or limit edituserjs and similar rights from users with "higher" access than the editor.Dec 9 2019, 11:46 PM

So to move forward on this - is there actually a current way to define that one "user ... has more permission" than another? What would this even be based on? Being able to add a user to "group x" can be added to any group. If this is not done, is there a task for it (which would be blocking this one)?

The fact that now user's js/css pages can be edited by this user and (interface) admins means that there are one more protection level, "me and admins". It's useful in some cases, for example, configuraton files of some of my bots and request pages for bot-executed requests for another my bots are .css files in my subspace, so, they can be edited by me and admins (request pages was created in 2016-2017, when any admin could edit this pages). It's useful opportunity, so, if we remove a right to edit this pages from even interface admins, it may be good if we explicitly establish a new protection level, "me and admins". Or change the behaviour of MediaWiki engine, so user can edit fully protected page if it is in this user's subspace.

So to move forward on this - is there actually a current way to define that one "user ... has more permission" than another?

Not really. Maybe it could be done as part of T208477: Move "privileged account' concept into MediaWiki core somehow.