Page MenuHomePhabricator

Moving user subpages is not rate limited
Closed, DeclinedPublic

Description

The option to "Move all subpages, if applicable" does not perform a rate-limiting check, which allows pagemove vandals to exploit this option and move all of a user's subpages.


Version: 1.13.x
Severity: normal
URL: http://en.wikipedia.org/w/index.php?title=Special:Log&user=NKbot

Details

Reference
bz14356

Event Timeline

bzimport raised the priority of this task from to Lowest.Nov 21 2014, 10:11 PM
bzimport set Reference to bz14356.
bzimport added a subscriber: Unknown Object (MLST).

Created attachment 4939
Patch to add rate limiting for moving/talk subpages

Adds a rate-limit-exceeded notice for each page that couldn't be moved in the list of pages that were supposed to be moved rather than $wgOut->rateLimited() so the user can still see the list of pages that were moved successfully (until they hit the limit) and which failed (due to the limit or other reasons).

Attached:

Created attachment 4942
Alternate patch - move basepage last

Splarka on IRC suggested that if a rate limit is tripped, the base page shouldn't be moved so that remaining sub/talk pages can be moved using the "move subpages" and "move talk page" options on the base page. This patch changes the MovePage form submission to try to move subpages and talk pages first, if applicable, then the base page if no rate limits are tripped. I tested it in various situations:

  1. Limit hit before all subpages can be moved:
  2. Subpages moved as long as the rate limiter allows
  3. Base page not moved
  1. Rate limit hit before anything can be moved:
  2. Uses standard $wgOut->rateLimited();
  1. No rate limits hit:
  2. Same action as it currently does
  1. Subpages moved, rate limit hit when moving base page:
  2. Same as #1, but the base page is the only one not moved

**Note: For some reason, the subpages of user talk pages are moved into project-space. This doesn't happen with user pages, it seems to happen with the current code as well, might be related to bug 14258.

attachment patch2.patch ignored as obsolete

Created attachment 4943
Fix for previous patch

Fix docs on previous patch - "Alternate patch - move basepage last"

Attached:

This would just mean that there's no practical way to move pages with their subpages.

mike.lifeguard+bugs wrote:

If there is concern this will be abused, we could consider making the subpages-too option a sysop-only right.

Hmm... how about queueing the subpage moves for delayed background execution using something like the job queue (though they should probably have their own separate queue) and (and this is the important bit) having the queue handler re-check that the user who initiated the move is still allowed to make it. That would allow anti-vandal bots to interfere with massive subpage moves before they run to completion. I know, probably not trivial to implement, but one can wish...

ayg wrote:

(In reply to comment #5)

If there is concern this will be abused, we could consider making the
subpages-too option a sysop-only right.

That might be a good idea, if only for the sake of the log spam. Or at least make it autoconfirmed. (It should be available to all users by default, though, since on a typical wiki there won't be enough pages with many subpages to make this harmful.)

(In reply to comment #6)

Hmm... how about queueing the subpage moves for delayed background execution
using something like the job queue (though they should probably have their own
separate queue) and (and this is the important bit) having the queue handler
re-check that the user who initiated the move is still allowed to make it.
That would allow anti-vandal bots to interfere with massive subpage moves
before they run to completion.

Seems ineffective. Besides, in the long run it may be more efficient to move all of the pages in a single query.

charlottethewebb wrote:

If there is concern this will be abused, we could consider making the
subpages-too option a sysop-only right.

That might be a good idea, if only for the sake of the log spam. Or at least
make it autoconfirmed. (It should be available to all users by default,
though, since on a typical wiki there won't be enough pages with many subpages
to make this harmful.)

One must already "autoconfirmed" to move individual pages (at least on enwiki -- default settings are probably more lax) so this would have no effect on vandalism.

Log spam might be better avoided by collapsing a "page and sub-pages" move into a single log action (expandable for those who wish to see the whole list). A quid pro quo "(revert all)" link would ensure that bad multi-page moves are entirely corrected, as would a button to delete all the redirects in one fell swoop, though these ideas should probably go on a separate ticket.........

ayg wrote:

(In reply to comment #8)

Log spam might be better avoided by collapsing a "page and sub-pages" move into
a single log action (expandable for those who wish to see the whole list). A
quid pro quo "(revert all)" link would ensure that bad multi-page moves are
entirely corrected, as would a button to delete all the redirects in one fell
swoop, though these ideas should probably go on a separate ticket.........

The log format is pretty inflexible at present, unfortunately, but what you say is probably the best interface, yes. Another bug would be good.