Deletion of user js and css requires deletion and edituser* rights
Closed, ResolvedPublic

Description

If you have delete, but not the right to edit such a page, you can't delete it.

This might be expected, but can lead into one problem: With the new group of people who can edit only these pages, they are only a few people who are a) admin and b) editor for css/js.

Means, if css or js gets abused to store abusive content or personal data, people are not able to delete.

Which means, that at least oversighter in all wikis need this right. Please take a look at this problem.

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 23 2018, 6:35 AM

Adding codebase project MediaWiki-User-management so those MediaWiki-User-management developers who also have access to Security tasks could get aware of this task (either on their project workboard or via Phab notifications). For future reference, you are welcome to add a corresponding codebase project to Security tasks - thank you!

Adding codebase project MediaWiki-User-management so those MediaWiki-User-management developers who also have access to Security tasks could get aware of this task (either on their project workboard or via Phab notifications). For future reference, you are welcome to add a corresponding codebase project to Security tasks - thank you!

it's easy to forget it, since when I create a task via the form, it's a locked field.

Reedy renamed this task from Persons without editusercss etc. but with can't delete pages to Deletion of user js and css requires deletion and edituser* rights.Jul 23 2018, 11:19 AM
jrbs added a subscriber: jrbs.Jul 23 2018, 12:27 PM
Luke081515 added a subscriber: Tgr.Jul 23 2018, 4:55 PM

The problem I see is, that, if you grant edituser* to oversight, which is needed imo, then you still have the problem, that on most wikis not all admins get the new group with edituser*, so they are less people that can take action on this pages, which makes it easier for trolls for example to use these pages to publish abusive content.

Tgr added a comment.Jul 23 2018, 8:15 PM

A similar concern is that in the case of an ongoing attack it might result in faster incident response if administrators could delete the malicious page themselves.

It would be nice to special-case deletion (I don't think it can be abused, security-wise) but I don't think there is an easy way as you can't tell that the permission error was due to one specific permission missing.

The problem is as well, that if you allow admins only to delete such pages, it's not possible to do revision deletion, since you are not able to revert the last version. But at least allow admins to delete such pages would make it possible to solve the problem temporary, until people with admin and cssjs rights can fix the situation.


That's the current error msg. Is it possible to enable a check, that passes this check if the action is delete, and the user is able to delete?

Tgr added a comment.Jul 24 2018, 9:46 AM

The problem is, it is not really possible to tell why an action has failed (short of trying to recognize the error message, which is super fragile). It could have been lack of editsitejs, another permission, a block, an extension hook, whatever.

I tried to find out why do we prevent users from deleting pages if they can't edit them, and I found that the check was added in 002a27790137e91b124ffeed6efc8dbe833e1dab, after it was discovered that superprotection did not prevent administrators from deleting and undeleting the page (which clears its protection settings: T14343), and then adjusted in 9dafa73b2f284ed570b7e4aa377115eb9271b5fc due to T71380 and T71398.

Since the ability to superprotect pages was removed, I think we're not depending on this behavior currently (at least on Wikimedia wikis). In my opinion this could be removed, and if we ever need to protect pages so that administrators can't delete them, I think a better solution would to be add 'delete' to $wgRestrictionTypes, which should allow delete-protecting pages (just like you can move-protect them currently).

Anomie added a subscriber: Anomie.Jul 24 2018, 3:47 PM

(at least on Wikimedia wikis)

"superprotect" wasn't anything special from a MediaWiki standpoint, it was just a configuration to add a protection level that normal admins didn't have the ability to edit. It's entirely possible a third-party wiki has configuration with hierarchical levels of protection which a delete-undelete would bypass in the same way, we should probably avoid reopening a security hole for them just because WMF doesn't currently have protection levels that would be affected by it.

matmarex removed a subscriber: matmarex.Jul 24 2018, 3:59 PM

@Tgr But in general: Do you agree that we should solve this issue, before we remove the edit rights for usercss/js from admins?

Tgr added a comment.Aug 2 2018, 12:43 PM

@Luke081515 if there is an easy solution, we should make sure it is in place by then. I don't see one though, and I think deploying the new permission system without one is OK - there have always been fringe ways of publishing abusive content that only a few users had the permission to fix (e.g. usernames) and it did not cause much disruption.

If this turns out to be a serious problem in practise, we can limit JS/CSS editing to autoconfirmed users or something like that.

Shouldn't we then at least add the permission to edit css/js to oversighters?

Luke081515 added a subscriber: MBq.Aug 4 2018, 11:51 AM
Tgr changed the visibility from "Custom Policy" to "Public (No Login Required)".

There isn't anything particularly secret about this and in any case it's being discussed in a number of public places so let's open it up.

I'm not seeing a huge issue here. Wikis will just need to ensure they have enough sysop + intadmins to delete these pages if they come up (if needed). If the issue of users creating oversightable content in a js/css page exists in the future, we could look at a workaround for oversighters. Otherwise stewards exist as always to act in emergencies or where nobody else can.

As there is already special checks in place for userjs/usercss could this also allow for deletion - as the mediawiki namespace normally has its own protection it won't allow arbitrary creation (unlike userspace).

Tgr added a comment.Aug 5 2018, 9:53 PM

Reading rMW9dafa73b2f28: Test only against protection for deleting more closely, I don't think it requires edit rights for deletion. It only requires that the user is not prevented from editing by a page protection.

I tested it earlier today - without the editsitecss/editsitejs you can't delete the page, even if it is unprotected.

I tested it earlier today - without the editsitecss/editsitejs you can't delete the page, even if it is unprotected.

And change contentmodel to something else, so you can delete it? Or move it to template namespace, for example, and then delete?

RP88 added a subscriber: RP88.Aug 6 2018, 1:33 AM

Change 455481 had a related patch set uploaded (by Gergő Tisza; owner: Gergő Tisza):
[mediawiki/core@master] Allow admins to delete user JS/CSS pages

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

stjn added a subscriber: stjn.Aug 27 2018, 2:03 PM

If there’s any way to also include undelete, it should probably be done if we’re allowing deletion of those pages.

Change 455481 merged by jenkins-bot:
[mediawiki/core@master] Allow admins to delete user JS/CSS pages

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

If there’s any way to also include undelete, it should probably be done if we’re allowing deletion of those pages.

I don't know. One can argue that creating a JS/CSS page is more impactful than deleting it, and the same for protected namespaces and areas where for instance you could be restoring some outdated information. Usually admin actions are "symmetrical" but for instance adding or removing user rights is not.

I think "delete" alone should be enough here, e.g. if there is abusive js in some place and the page gets deleted, you don't want it restored by people that can't make it normally.

Tgr added a comment.Aug 27 2018, 10:43 PM

Yeah, people who can't edit a page shouldn't be able to create them by other means either. Delete + selective undelete can be used to revert to an older revision, and that could be used to restore a fixed security vulnerability in someone's personal JS, for example.

stjn added a comment.Aug 27 2018, 10:44 PM

Alright, disregard.

Tgr closed this task as Resolved.Aug 27 2018, 11:09 PM
Tgr claimed this task.