Page MenuHomePhabricator

Defect: API action=block allows blocking non-existent accounts
Closed, ResolvedPublic

Description

This is a somewhat incomplete report because I'm not really sure what happened. When trying to block the account named Ks0s got STUNG due to INCOMPETENCE of CIDR\17 block on enwp, Oshwah encountered some trouble with the Special:Contributions UI thinking it was an IP range, and ended up trying to block via the API.

Somehow they blocked https://en.wikipedia.org/wiki/Special:Contributions/Ks0s_got_STUNG_due_to_INCOMPETENCE_of_CIDR (a nonexistent account, has ipb_user=0 in the db), and the actual account https://en.wikipedia.org/wiki/Special:Contributions/Ks0s_got_STUNG_due_to_INCOMPETENCE_of_CIDR%5C17_block

These are blocks 8081998 and 8081999 in the enwiki database. I'm not sure if there's something wrong with how we handle usernames with \ in them, so marking security out of caution. There does seem to be an API bug in that it was possible to block a non-existent account?

Event Timeline

The first block was an attempt to block "Ks0s got STUNG due to INCOMPETENCE of CIDR/17 block" instead, with a forward slash instead of a backslash. From the API log file:

2017-12-19 00:47:37 [WjhhqQpAMCIAAE9OseMAAAAU] mw1202 enwiki 1.31.0-wmf.12 api INFO: API POST Oshwah [redacted] T=49ms action=block format=json user=Ks0s%20got%20STUNG%20due%20to%20INCOMPETENCE%20of%20CIDR/17%20block reason=%5B%5BWP:SOCK%7Csock%20puppetry%5D%5D nocreate=true autoblock=true noemail=true token=[redacted]

The API rejects usernames that are valid but refer to nonexistent users and usernames that are valid but unusable. It does not, however, reject usernames that are invalid, including IP addresses and names with forward slashes in them. Instead, it passes them to the backend code in SpecialBlock, which gets into Block::parseTarget() to determine the actual target. And that strips anything after a forward slash with a comment referring to T31797.

OK, thank you for figuring that out :) I +2'd https://gerrit.wikimedia.org/r/#/c/399190/ and I'd like to backport that if it cherry-picks cleanly. If there's no security issue I think we can make this task public?

I don't see any need for this task to be private.

Legoktm renamed this task from Ability to block non-existent account and potential weirdness with usernames with \ in them to API action=block allows blocking non-existent accounts.Dec 22 2017, 6:02 AM
Legoktm changed the visibility from "Custom Policy" to "Public (No Login Required)".
Legoktm removed a project: acl*security.
TBolliger renamed this task from API action=block allows blocking non-existent accounts to Defect: API action=block allows blocking non-existent accounts.Jan 12 2018, 11:07 PM

@Legoktm: Are you still planning to backport, or should we close this?

@Legoktm: ping - could you answer the last question please? TIA

Legoktm claimed this task.

My bad, probably not worth it anymore since it's included in the (now) latest LTS.