PrivateDomains' form at Special:PrivateDomains allows to effectively edit a bunch of pages in NS_MEDIAWIKI which are used by PrivateDomains to store its settings on-wiki...buuuuuuut there's no anti-CSRF check implemented on the special page, all it cares about is that the request was POSTed and that action is submit.
Description
Description
Details
Details
- Risk Rating
- Medium
- Author Affiliation
- Wikimedia Communities
Related Objects
Related Objects
Event Timeline
Comment Actions
diff --git a/includes/SpecialPrivateDomains.php b/includes/SpecialPrivateDomains.php index 2b6c2ef..f13f444 100644 --- a/includes/SpecialPrivateDomains.php +++ b/includes/SpecialPrivateDomains.php @@ -67,18 +67,24 @@ class SpecialPrivateDomains extends SpecialPage { */ public function execute( $par ) { $request = $this->getRequest(); + $user = $this->getUser(); $this->setHeaders(); $msg = ''; if ( $request->wasPosted() ) { - if ( $request->getText( 'action' ) == 'submit' ) { + $tokenOk = $user->matchEditToken( $request->getVal( 'wpEditToken' ) ); + if ( !$tokenOk ) { + $msg = $this->msg( 'sessionfailure' )->parse(); + } + + if ( $request->getText( 'action' ) == 'submit' && $tokenOk ) { $this->saveParam( 'privatedomains-domains', $request->getText( 'listdata' ) ); $this->saveParam( 'privatedomains-affiliatename', $request->getText( 'affiliateName' ) ); $this->saveParam( 'privatedomains-emailadmin', $request->getText( 'optionalPrivateDomainsEmail' ) ); - $msg = $this->msg( 'saveprivatedomains-success' )->text(); + $msg = $this->msg( 'saveprivatedomains-success' )->escaped(); } } @@ -119,13 +125,14 @@ class SpecialPrivateDomains extends SpecialPage { // Render the main form for changing PrivateDomains' settings. $out->addHTML( '<form name="privatedomains" id="privatedomains" method="post" action="' . $action . '"> - <label for="affiliateName"><br />' . $this->msg( 'privatedomains-affiliatenamelabel' )->text() . ' </label> + <label for="affiliateName"><br />' . $this->msg( 'privatedomains-affiliatenamelabel' )->escaped() . ' </label> <input type="text" name="affiliateName" width="30" value="' . $this->getParam( 'privatedomains-affiliatename' ) . '" /> - <label for="optionalEmail"><br />' . $this->msg( 'privatedomains-emailadminlabel' )->text() . ' </label> + <label for="optionalEmail"><br />' . $this->msg( 'privatedomains-emailadminlabel' )->escaped() . ' </label> <input type="text" name="optionalPrivateDomainsEmail" value="' . $this->getParam( 'privatedomains-emailadmin' ) . '" />' ); $out->addWikiMsg( 'privatedomains-instructions' ); $out->addHTML( '<textarea name="listdata" rows="10" cols="40">' . $this->getParam( 'privatedomains-domains' ) . '</textarea>' ); - $out->addHTML( '<br /><input type="submit" name="saveList" value="' . $this->msg( 'saveprefs' )->plain() . '" />' ); + $out->addHTML( Html::hidden( 'wpEditToken', $user->getEditToken() ) ); + $out->addHTML( '<br /><input type="submit" name="saveList" value="' . $this->msg( 'saveprefs' )->escaped() . '" />' ); $out->addHTML( '</form>' ); }
Quick patch which also adjusts the escaping of several UI messages used by the form because might as well fix that while I'm editing this portion of the code. (Also I'm too lazy to split out the changes, so...)
Comment Actions
@ashley - Seeing as how this was merged to master in gerrit, we can likely make this task public, correct? Or would other, affected mediawiki operators still prefer it remain protected for the time being?
Comment Actions
Right, sorry, I meant to poke @Reedy about that on IRC but forgot; please feel free to do so, since that's really all that's left here, the bug is effectively resolved and the fix has been made available.