Maniphest T203885

Special:NovaSudoer does not escape usernames
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	Bawolff
	Sep 8 2018, 5:05 PM

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

	Subject	Repo	Branch	Lines +/-
	SECURITY: Fix escaping of "runas" users on Special:NovaSudoer	mediawiki/extensions/OpenStackManager	master	+18 -8
	SECURITY: Escape usernames on Special:NovaSudoer	mediawiki/extensions/OpenStackManager	master	+6 -6

Customize query in gerrit

Event Timeline

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 project: phan-taint-check-plugin.Sep 8 2018, 5:06 PM

Bawolff added a subscriber: Umherirrender.

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

Special:NovaSudoer XSS1 KBDownload

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.

In T203885#4570310, @Legoktm wrote:

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

ReleaseTaggerBot added a project: MW-1.32-notes (WMF-deploy-2018-09-18 (1.32.0-wmf.22)).Sep 11 2018, 12:00 AM

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 "acl*security (Project)".

Bawolff changed Risk Rating from default to Medium.

So looks like I missed a case:

T203885-2.patch3 KBDownload

PS2:

T203885-2-1.patch3 KBDownload

In T203885#4579195, @Bawolff wrote:

PS2:

T203885-2-1.patch3 KBDownload

LGTM

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 "acl*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

ReleaseTaggerBot added a project: MW-1.33-notes (1.33.0-wmf.4; 2018-11-13).Nov 8 2018, 7:00 AM

sbassett moved this task from Backlog to Done on the phan-taint-check-plugin board.Oct 15 2019, 7:04 PM

sbassett triaged this task as Medium priority.Oct 15 2019, 7:19 PM

• chasemp added a project: Security.Feb 10 2020, 10:58 PM

• chasemp removed a project: acl*security.Feb 20 2020, 8:17 PM