Page MenuHomePhabricator

Modified HTTP headers allow XSS in SecurePoll (CVE-2021-42045)
Closed, ResolvedPublicSecurity

Description

  1. Create a simple poll
  2. Set your User-Agent: or X-Forwarded-For: header to <img src="foo.jpg" onerror="alert('XSS')">
  3. Submit a vote
  4. From an electionadmin account, view the list of votes (Special:SecurePoll/list/xxx)

An alert will appear. Tested with SecurePoll version 3.0.0 (ab0903e), MediaWiki 1.37.0-alpha (e56ca63)

Related Objects

Event Timeline

Legoktm triaged this task as Unbreak Now! priority.Aug 20 2021, 10:44 PM
Legoktm added subscribers: tstarling, Niharika, STran, Legoktm.

Thanks for reporting this @suffusion_of_yellow, this is pretty bad.

Attached a quick patch that I haven't tested yet.

Urbanecm lowered the priority of this task from Unbreak Now! to High.Aug 20 2021, 10:44 PM
Urbanecm added a project: Anti-Harassment.
Urbanecm added a subscriber: Urbanecm.

This can be a very privacy-impactful bug, considering that electionadmins have access to CU-like data for all voters. Since we have a large-scale election, this needs to be fixed ASAP.

Urbanecm raised the priority of this task from High to Unbreak Now!.Aug 20 2021, 10:45 PM

Thanks for reporting this @suffusion_of_yellow, this is pretty bad.

Attached a quick patch that I haven't tested yet.

Tested and approved.

Oof, thanks for reporting @suffusion_of_yellow and fixing @Legoktm and testing @Urbanecm. How quickly can we get this deployed?

Deploying the patch now after discussing with @Niharika. The main risk is that some output on the voter list page is double-escaped and garbled, which is still better than an active XSS.

I git blamed as well, for reference this was introduced in 86c92d32 (2013-01-22).

I reviewed the remaining pagers, and found two unescaped messages in SecurePollLogPager. This should not be exploitable on votewiki though thanks to the locked down setup there.

@Legoktm I just saw your question on IRC about why taint-check didn't catch this; I was curious and I'm replying here so it doesn't get lost. The root cause is the same as T289067#7296339: that formatValue method is only called in TablePager::formatRow, but taint-check doesn't know about the taintedness of methods declared in subclasses. So if you copy formatRow to ListPager AND add echo $this->formatValue( $field, $value ) in there you'll get an XSS warning. But then formatRow is still only called in IndexPager::getBody, which is called by TablePager::getBodyOutput, which is what's used by ListPage, so you'd have to copy all of them to get an actual XSS warning for the current code. Then I'm not sure if taint-check can understand OutputPage::addParserOutputContent / ::addParserOutputText, but that's not the main problem here.

Since it's the second times in a couple of days that subclass methods are causing an XSS, I've just filed https://github.com/phan/phan/issues/4502 upstream. Also noting that T289314 is the relevant task against phan-taint-check-plugin.

sbassett lowered the priority of this task from Unbreak Now! to Low.Aug 23 2021, 3:11 PM
sbassett added a subscriber: sbassett.

Lowering from UBN since @Legoktm deployed the patch to production.

sbassett changed the visibility from "Custom Policy" to "Public (No Login Required)".Oct 1 2021, 8:17 PM
sbassett changed the edit policy from "Custom Policy" to "All Users".
sbassett changed Risk Rating from N/A to Low.

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

[mediawiki/extensions/SecurePoll@master] SECURITY: Fix XSS via User-agent or XFF header on voter list

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

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

[mediawiki/extensions/SecurePoll@REL1_37] SECURITY: Fix XSS via User-agent or XFF header on voter list

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

Change 725368 merged by jenkins-bot:

[mediawiki/extensions/SecurePoll@master] SECURITY: Fix XSS via User-agent or XFF header on voter list

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

Change 725280 merged by jenkins-bot:

[mediawiki/extensions/SecurePoll@REL1_37] SECURITY: Fix XSS via User-agent or XFF header on voter list

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

sbassett closed this task as Resolved.EditedOct 1 2021, 8:34 PM
sbassett assigned this task to Legoktm.
sbassett moved this task from Watching to Our Part Is Done on the Security-Team board.

Note: the patches from T289385#7298822 and T289385#7298925 were squashed into the gerrit change sets.

Mstyles renamed this task from Modified HTTP headers allow XSS in SecurePoll to Modified HTTP headers allow XSS in SecurePoll (CVE-2021-42045).Oct 7 2021, 8:35 PM