Page MenuHomePhabricator

Exposed suppressed username via Special:Redirect
Open, HighPublic

Description

  • Problem: Special:Redirect/user/NNN does not check target user_id's suppressed status.
  • Step to reproduce :
    1. Go to Special:Redirect
    2. Make sure Lookup: type is User ID
    3. Entry Value: in optimal numbers. e.g. increase user_id until finding not listed entry at Special:ListUsers
  1. Directly link as [[Special:Redirect/user/SUPPRESSED_USER_ID]] replace SUPPRESSED_USER_ID for optimal numbers.
  2. Actual Results: Redirect to suppressed user's user page .
  3. Expected Results: Forbid redirecting to suppressed username.

Event Timeline

Rxy created this task.Aug 13 2019, 9:59 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 13 2019, 9:59 AM
Rxy added subscribers: sbassett, Reedy.

I wrote a patch for this ticket.

Rxy moved this task from Backlog to Security on the User-Rxy board.Aug 13 2019, 10:09 AM
Rxy added a subscriber: Ohgi.Aug 13 2019, 12:08 PM
sbassett triaged this task as High priority.Aug 13 2019, 2:44 PM
sbassett updated the task description. (Show Details)Aug 13 2019, 2:49 PM
sbassett updated the task description. (Show Details)
sbassett added a comment.EditedAug 13 2019, 7:18 PM

@Rxy - a couple of thoughts:

  1. So this patch just prevents a Special:Redirect based upon the numerical user_id of a hidden user, correct? I believe the User: pages of hidden users are still publicly-viewable if they exist (i.e. they don't display the User account "xxx" is not registered. error message.) The intention here is to throw an error if anyone tries to Special:Redirect to them, to prevent some information disclosure, correct?
  2. I'm not sure the hideuser permissions error message makes sense within the context of a Special:Redirect:
You do not have permission to block a username, hiding it from the public...

as it seems to be more directed at users with that permission as opposed to someone trying to visit a redirect. I agree that's the right permission to check but I might recommend something like this for throwing a more relevant error message:

throw new ErrorPageError( 'badaccess-group0', 'badaccess-group0' );

Otherwise, I think this seems fine and I can security-deploy it whenever, though honestly I'd view this more as a hardening measure that could probably be done in gerrit.

Rxy added a comment.Aug 13 2019, 10:43 PM
  1. Yes, It is correct. If who knew hideuser-ed user name, they can directly access to user page or they can check account existence via trying account creation. I guess no need to shown message User account "xxx" is not registered.. The patch is prevent leaks hideuser-ed account name who aren't know hideuser-ed account name by trying increase user_id.
  1. Here is Patch Set 2:

Thanks for reviewing.

PS2 looks good to me. I can try to deploy this later today or tomorrow, since there's no train this week.

I'm deploying this now through gerrit. Will make this task public once deployed.

sbassett changed the visibility from "Custom Policy" to "Public (No Login Required)".Aug 15 2019, 7:22 PM

Change 530441 merged by jenkins-bot:
[mediawiki/core@REL1_32] SECURITY: Add permission check for suppressed account

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

https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/530440/ needs another CR+2 to tell Jenkins to merge - original gate-and-submit failed verification

Change 530443 merged by jenkins-bot:
[mediawiki/core@REL1_33] SECURITY: Add permission check for suppressed account

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

Hmm, this is the second issue with revdel & Special:Redirect :( [The other one being T187638]. Guess we need to be careful with this page

Change 530440 merged by Jforrester:
[mediawiki/core@REL1_31] SECURITY: Add permission check for suppressed account

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

Is there anything further to do on this?