r77031 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r77030‎ | r77031 | r77032 >
Date:21:02, 19 November 2010
Author:mah
Status:resolved (Comments)
Tags:
Comment:
followup r76848 by using wfEscapeWikiText() on all bits except links. Escape links using wfEscapeWikiText() that, after we've parsed them with parse_url(), don't have a scheme in our list of validSchemes.
Modified paths:
  • /trunk/extensions/RSS/RSSParser.php (modified) (history)

Diff [purge]

Index: trunk/extensions/RSS/RSSParser.php
——@@ -13,6 +13,7 @@
1414 protected $xml;
1515 protected $error;
1616 protected $displayFields = array( 'author', 'title', 'encodedContent', 'description' );
 17+ protected $validScheme = array( 'http', 'https', 'ftp' );
1718
1819 public $client;
1920
——@@ -241,8 +242,8 @@
242243 protected function renderItem( $item, $parser, $frame ) {
243244 $output = "";
244245 if ( isset( $parser ) && isset( $frame ) ) {
245 - $displayFields = array_flip( $this->displayFields );
246246 $rendered = $this->itemTemplate;
 247+ $validScheme = array_flip( $this->validScheme );
247248
248249 // $info will only be an XML element name, so we're safe
249250 // using it. $item[$info] is handled by the XML parser --
——@@ -250,14 +251,21 @@
251252 // <description><script>alert("hi")</script></description> will find its
252253 // rogue <script> tags neutered.
253254 foreach ( array_keys( $item ) as $info ) {
254 - if ( isset( $displayFields[ $info ] ) ) {
 255+ if ( $info != 'link' ) {
255256 $txt = $this->highlightTerms( wfEscapeWikiText( $item[ $info ] ) );
256257 } else {
257 - $txt = wfEscapeWikiText( $item[ $info ] );
 258+ $url = $item[ $info ];
 259+ $scheme = parse_url( $url, PHP_URL_SCHEME );
 260+ if( isset( $validScheme[$scheme] ) ) {
 261+ $txt = $url;
 262+ } else {
 263+ $txt = wfEscapeWikiText( $url );
 264+ }
258265 }
259266 $rendered = str_replace( '{{{' . $info . '}}}', $txt, $rendered );
260267 }
261 - $output .= $parser->recursiveTagParse( $rendered, $frame );
 268+
 269+ $output = $parser->recursiveTagParse( $rendered, $frame );
262270 }
263271 return $output;
264272 }

Follow-up revisions

RevisionCommit summaryAuthorDate
r77128* Avoid XSS via malicious RSS on wikis with $wgRawHtml = true. Mostly reverts...tstarling02:16, 23 November 2010

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r76848* Remove references to unused charset handling....mah23:21, 16 November 2010

Comments

#Comment by Tim Starling (talk | contribs)   01:10, 23 November 2010

That doesn't solve anything. 

> var_dump( parse_url( 'http://<html><script>alert("hello")</script></html>' ) )
array(3) {
  ["scheme"]=>
  string(4) "http"
  ["host"]=>
  string(29) "<html><script>alert("hello")<"
  ["path"]=>
  string(15) "/script></html>"
}

> var_dump( parse_url( 'http://<html><script>alert("hello")</script></html>', PHP_URL_SCHEME ) )
string(4) "http"

How about I just do it myself?

Status & tagging log