Page MenuHomePhabricator

CVE-2022-34911: Username is not escaped in the "welcomeuser" message
Closed, ResolvedPublicSecurity

Description

If you create an account with some HTML in its name (which is not allowed by default, see T308465), after the account creation when it sets the page title to "Welcome, [username]" (using the welcomeuser), the username is not escaped: SpecialCreateAccount::successfulAction() calls ::showSuccessPage() with a message as second parameter, and OutputPage::setPageTitle() uses text().

Filing as a security task out of an abundance of caution.

Event Timeline

sbassett added a subscriber: sbassett.

Untested patch that should fix the issue:


I can't imagine we'd want to change LoginSignupSpecialPage->showSuccessPage for any reason. Similar to T308473, I'd imagine this could just go through gerrit.

sbassett added a project: user-sbassett.
sbassett moved this task from Incoming to In Progress on the Security-Team board.
sbassett moved this task from Backlog to In Progress on the user-sbassett board.
sbassett changed Risk Rating from N/A to Low.

Change 805208 had a related patch set uploaded (by SBassett; author: SBassett):

[mediawiki/core@master] SECURITY: Escape welcomeuser message passed to showSuccessPage()

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

sbassett changed the visibility from "Custom Policy" to "Public (No Login Required)".Jun 13 2022, 5:17 PM
sbassett changed the edit policy from "Custom Policy" to "All Users".

But OutputPage::setPageTitle() calls Sanitizer::removeSomeTags, is that not enough?

But OutputPage::setPageTitle() calls Sanitizer::removeSomeTags, is that not enough?

For a lot of common XSS vectors, sure. But it'll allow lower-level obfuscations and similar, minor CSS hacks, like: <span style=display:none>username</span>. I'm also going to guess that this bug was possibly found via phan-taint-check-plugin which I'm not certain adjusts any taints for Sanitizer::removeSomeTags. Which are all reasons why this is a very low-risk bug and really mostly code-hardening.

Change 805208 merged by jenkins-bot:

[mediawiki/core@master] SECURITY: Escape welcomeuser message passed to showSuccessPage()

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

But OutputPage::setPageTitle() calls Sanitizer::removeSomeTags, is that not enough?

For a lot of common XSS vectors, sure. But it'll allow lower-level obfuscations and similar, minor CSS hacks, like: <span style=display:none>username</span>.

Yeah, better be safe. FWIW, "<" is not allowed in usernames unless you change the config, and ">" is forbidden regardless of config because it's reserved for external usernames.

I'm also going to guess that this bug was possibly found via phan-taint-check-plugin which I'm not certain adjusts any taints for Sanitizer::removeSomeTags.

Actually... I created an account with "<" in its name for unrelated tests, and though I'd explore the interface a bit just to make sure that there was no obvious XSS around.

Change 807005 had a related patch set uploaded (by SBassett; author: SBassett):

[mediawiki/core@REL1_38] SECURITY: Escape welcomeuser message passed to showSuccessPage()

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

Change 807146 had a related patch set uploaded (by SBassett; author: SBassett):

[mediawiki/core@REL1_37] SECURITY: Escape welcomeuser message passed to showSuccessPage()

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

Change 807147 had a related patch set uploaded (by SBassett; author: SBassett):

[mediawiki/core@REL1_35] SECURITY: Escape welcomeuser message passed to showSuccessPage()

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

This picked cleanly to all supported release branches. If those test fine, I'll plan to merge them and this task can be resolved.

Change 807147 merged by jenkins-bot:

[mediawiki/core@REL1_35] SECURITY: Escape welcomeuser message passed to showSuccessPage()

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

Change 807146 merged by jenkins-bot:

[mediawiki/core@REL1_37] SECURITY: Escape welcomeuser message passed to showSuccessPage()

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

Change 807005 merged by jenkins-bot:

[mediawiki/core@REL1_38] SECURITY: Escape welcomeuser message passed to showSuccessPage()

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

sbassett claimed this task.
sbassett moved this task from In Progress to Our Part Is Done on the Security-Team board.
sbassett moved this task from In Progress to Done on the user-sbassett board.
sbassett removed a project: Patch-For-Review.
Reedy renamed this task from Username is not escaped in the "welcomeuser" message to CVE-2022-34911: Username is not escaped in the "welcomeuser" message.Jul 2 2022, 7:40 PM