Special:NovaSudoer does not escape usernames
Closed, ResolvedPublic

Description

Not sure how serious that is (Can usernames/servicenames contain < or >. Normal mw names cannot). Erring on the side of caution

discovered by phan taint check

Details

Risk Rating
Medium
Bawolff created this task.Sep 8 2018, 5:05 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 8 2018, 5:05 PM
Bawolff added a subscriber: Reedy.

I would like to apply on monday. Appreciate if someone could "+1" it on the task

Pretty sure they cannot contain <>. From https://wikitech.wikimedia.org/wiki/Special:CreateAccount

... It must start with a-z, and can only contain lowercase a-z, 0-9 and - characters.

+1 to the patch.

Pretty sure they cannot contain <>. From https://wikitech.wikimedia.org/wiki/Special:CreateAccount

... It must start with a-z, and can only contain lowercase a-z, 0-9 and - characters.

+1 to the patch.

They definitely can't in MediaWiki (<> or invalid in Titles, user's have to be valid Titles. Although the CreateAccount page seems wrong as my name has a space/underscore in). I was more not sure about new horizion stuff and what a "servicename" is.

Bawolff changed the visibility from "Custom Policy" to "Public (No Login Required)".Sep 10 2018, 9:33 PM

Change 459640 had a related patch set uploaded (by Brian Wolff; owner: Brian Wolff):
[mediawiki/extensions/OpenStackManager@master] SECURITY: Escape usernames on Special:NovaSudoer

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

Change 459640 merged by Brian Wolff:
[mediawiki/extensions/OpenStackManager@master] SECURITY: Escape usernames on Special:NovaSudoer

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

Bawolff closed this task as Resolved.Sep 11 2018, 4:33 AM
Bawolff claimed this task.
Bawolff changed the visibility from "Public (No Login Required)" to "No One".Sep 13 2018, 1:31 AM
Bawolff changed Risk Rating from N/A to default.
Bawolff changed the visibility from "No One" to "Security (Project)".
Bawolff changed Risk Rating from default to Medium.
Bawolff reopened this task as Open.Sep 13 2018, 1:34 AM

So looks like I missed a case:

PS2:

Reedy added a comment.Sep 13 2018, 1:56 AM

PS2:

LGTM

hashar added a subscriber: hashar.Sep 18 2018, 2:29 PM

Patch "SECURITY: Escape usernames on Special:NovaSudoer" is apparently merged and has made it into 1.32.0-wmf.22. I have dropped it from the cluster.

Bawolff changed the visibility from "Security (Project)" to "Public (No Login Required)".Nov 8 2018, 6:33 AM

Change 472380 had a related patch set uploaded (by Brian Wolff; owner: Brian Wolff):
[mediawiki/extensions/OpenStackManager@master] SECURITY: Fix escaping of "runas" users on Special:NovaSudoer

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

Change 472380 merged by Brian Wolff:
[mediawiki/extensions/OpenStackManager@master] SECURITY: Fix escaping of "runas" users on Special:NovaSudoer

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

Bawolff closed this task as Resolved.Nov 8 2018, 6:42 AM