The user Zabe (test 9) is suppressed on the beta cluster.
Still it is possible to discover its existence through Special:Impact.
Actual result
Expected result
Zabe | |
Dec 25 2021, 4:58 PM |
F34896154: 0001-SECURITY-Don-t-leak-suppressed-usernames-on-Special-.patch | |
Dec 25 2021, 4:59 PM |
F34896151: special_impact_expected.png | |
Dec 25 2021, 4:58 PM |
F34896147: zabe_9_suppressed.png | |
Dec 25 2021, 4:58 PM |
F34896149: special_impact_leak.png | |
Dec 25 2021, 4:58 PM |
The user Zabe (test 9) is suppressed on the beta cluster.
Still it is possible to discover its existence through Special:Impact.
Actual result
Expected result
Issue confirmed on enwiki and +1 to the patch above. Would be nice to get another +1 from GrowthExperiments folks (@Urbanecm_WMF @Tgr et al) prior to deployment.
Well, the fact that a known username is suppressed isn't really secret. You can always just check via list=users - some of that should probably be hidden but we need to tell new users at some point during the account creation process that the account already exists, so at least the cancreate flag needs to work (although I suppose we could try to make that check so late that you can't verify without not creating a new account if the username wasn't taken).
The patch looks good otherwise.
I don't think so. It just worked for you because I undid the suppression of my testing account after taking the screenshots. I redid the suppression now and if you now perform that query it also states missing:true like for nonexistent accounts.
Sorry, I wasn't clear about my point, which is that you can't fake the can-create check (or, you could, but it would result in erratic behavior):
https://en.wikipedia.beta.wmflabs.org/w/api.php?action=query&format=jsonfm&list=users&ususers=Zabe%20(test%209)&usprop=cancreate
"name": "Zabe (test 9)", "missing": "", "cancreateerror": [ { "message": "userexists", "params": [], "code": "userexists", "type": "error" } ]
23:38 <urbanecm> !log Deploy security patch for T298312 23:38 <+stashbot> Logged the message at https://wikitech.wikimedia.org/wiki/Server_Admin_Log
Deployed to production as-of F34896154. Works as expected. For a suppressed user, Special:Impact claims "Missing or invalid username." logged-out, but when logged-in under my WMF account (which has hideuser), Special:Impact gladly shows information about that user.
Moving back to Incoming for security-team to triage (and publish & merge to master, as appropriate).
Thanks, @Urbanecm_WMF. Now tracking for the supplemental release (T297839) and on the current deployments bug (T276237).
Change 775916 had a related patch set uploaded (by SBassett; author: Zabe):
[mediawiki/extensions/GrowthExperiments@master] SECURITY: Don't leak suppressed usernames on Special:Impact
Change 775927 had a related patch set uploaded (by SBassett; author: Zabe):
[mediawiki/extensions/GrowthExperiments@REL1_37] SECURITY: Don't leak suppressed usernames on Special:Impact
Change 775928 had a related patch set uploaded (by SBassett; author: Zabe):
[mediawiki/extensions/GrowthExperiments@REL1_36] SECURITY: Don't leak suppressed usernames on Special:Impact
Change 775929 had a related patch set uploaded (by SBassett; author: Zabe):
[mediawiki/extensions/GrowthExperiments@REL1_35] SECURITY: Don't leak suppressed usernames on Special:Impact
Change 775916 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@master] SECURITY: Don't leak suppressed usernames on Special:Impact
Change 776035 had a related patch set uploaded (by Zabe; author: Zabe):
[mediawiki/extensions/GrowthExperiments@REL1_38] SECURITY: Don't leak suppressed usernames on Special:Impact
Change 776035 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@REL1_38] SECURITY: Don't leak suppressed usernames on Special:Impact
Change 775927 merged by jenkins-bot:
[mediawiki/extensions/GrowthExperiments@REL1_37] SECURITY: Don't leak suppressed usernames on Special:Impact
Change 775929 merged by Umherirrender:
[mediawiki/extensions/GrowthExperiments@REL1_35] SECURITY: Don't leak suppressed usernames on Special:Impact
Change 775928 merged by Umherirrender:
[mediawiki/extensions/GrowthExperiments@REL1_36] SECURITY: Don't leak suppressed usernames on Special:Impact