Page MenuHomePhabricator

PollNY: Stored XSS (CVE-2020-29003)
Closed, ResolvedPublicSecurity


Like T266400 but only for PollNY this time around...

Prerequisites: social tools setup (MW 1.34 with SocialProfile, and for this particular bug, also need PollNY)

  1. Create a poll via Special:CreatePoll, or edit an existing one via Special:UpdatePoll
  2. Have an answer option contain something like <script>alert('XSS')</script>
  3. Save changes
  4. When viewing the poll's page in the NS_POLL namespace, the malicious code gets executed despite that it damn well shouldn't

The last step is also true for polls embedded on other wiki pages via the pollembed parser tag.

Event Timeline

Proposed patch which fixes the issues noted here and includes some unrelated no-JS work (T248390); the relevant chunks are obviously the ones where htmlspecialchars is mentioned, except for the last one ("next poll" URL stuff), that's strictly no-JS related and not related to this ticket.

@@ -230,8 +230,9 @@ class PollNYHooks {
 					foreach ( $poll_info['choices'] as $choice ) {
 						$output .= "<div class=\"poll-choice\">
-						<input type=\"radio\" name=\"poll_choice\" data-poll-id=\"{$poll_info['id']}\" data-poll-page-id=\"{$poll_page_id}\" id=\"poll_choice\" value=\"{$choice['id']}\">{$choice['choice']}
-						</div>";
+						<input type=\"radio\" name=\"poll_choice\" data-poll-id=\"{$poll_info['id']}\" data-poll-page-id=\"{$poll_page_id}\" id=\"poll_choice\" value=\"{$choice['id']}\">";
+						$output .= htmlspecialchars( $choice['choice'], ENT_QUOTES );
+						$output .= '</div>';
 					$output .= Html::submitButton( wfMessage( 'poll-submit-btn' )->escaped(), [ 'class' => 'poll-vote-btn-nojs' ] );

Do you want to apply the same (int) casting for these ids just to make it obvious these are safe? Or switch to the Html wrapper which should escape attributes I believe?

Other stuff LGTM.

@Legoktm Thanks for the CR! While attempting to implement your changes, I found a few more nasty stored XSS lines in that same's a new patch which hopefully catches 'em all, for good this time around.

This looks like it fixes the XSS's properly, as far as that patch goes (I did not look at any of the other PollNY code beyond what was in the patch)

+1 to the patch at T266508#6583137. One thing I did notice was that in PollNY.hooks.php, the $choice['percent'] variable used to build the html string on line 263 isn't cast to an int as similar variables are within the patch. The cast likely isn't necessary, even though this does appear to be a database value as opposed to a computed value as the similar percent variables are within PollPage.class.php. Just pointing out for consistency's sake.

Legoktm changed the visibility from "Custom Policy" to "Public (No Login Required)".
Legoktm changed the edit policy from "Custom Policy" to "All Users".
sbassett renamed this task from PollNY: Stored XSS to PollNY: Stored XSS (CVE-2020-29003).Dec 1 2020, 5:47 PM

Change 651588 had a related patch set uploaded (by SBassett; owner: Jack Phoenix):
[mediawiki/extensions/PollNY@REL1_35] [SECURITY] Fix stored XSS via poll choices on Poll: pages etc.

Change 651588 merged by jenkins-bot:
[mediawiki/extensions/PollNY@REL1_35] [SECURITY] Fix stored XSS via poll choices on Poll: pages etc.