tbleher(a)svn.wikimedia.org wrote:
Revision: 29651
Author: tbleher
Date: 2008-01-12 14:33:50 +0000 (Sat, 12 Jan 2008)
Log Message:
-----------
Security fix: escape all URLs, as they are written directly into the
HTML of the page.
Modified Paths:
--------------
trunk/extensions/ProofreadPage/ProofreadPage.php
Modified: trunk/extensions/ProofreadPage/ProofreadPage.php
===================================================================
--- trunk/extensions/ProofreadPage/ProofreadPage.php 2008-01-12 13:44:08 UTC (rev 29650)
+++ trunk/extensions/ProofreadPage/ProofreadPage.php 2008-01-12 14:33:50 UTC (rev 29651)
@@ -69,16 +69,16 @@
$prev_name = "$page_namespace:$name/" . ( $pagenr - 1 );
$next_name = "$page_namespace:$name/" . ( $pagenr + 1 );
$index_name = "$index_namespace:$name";
- $prev_url = ( $pagenr == 1 ) ? '' : Title::newFromText( $prev_name
)->getFullURL();
- $next_url = ( $pagenr == $count ) ? '' : Title::newFromText( $next_name
)->getFullURL();
- $index_url = Title::newFromText( $index_name )->getFullURL();
+ $prev_url = ( $pagenr == 1 ) ? '' : Title::newFromText( $prev_name
)->escapeFullURL();
+ $next_url = ( $pagenr == $count ) ? '' : Title::newFromText( $next_name
)->escapeFullURL();
+ $index_url = Title::newFromText( $index_name )->escapeFullURL();
return array( $index_url, $prev_url, $next_url );
}
return $err;
}
$index_title = $ref_title;
- $index_url = $index_title->getFullURL();
+ $index_url = $index_title->escapeFullURL();
$rev = Revision::newFromTitle( $index_title );
$text = $rev->getText();
@@ -93,13 +93,13 @@
if( ($i>0) && ($i<count($links[1])) ){
$prev_title = Title::newFromText( $links[1][$i-1] );
if(!$prev_title) return $err;
- $prev_url = $prev_title->getFullURL();
+ $prev_url = $prev_title->escapeFullURL();
}
else $prev_url = '';
if( ($i>=0) && ($i+1<count($links[1])) ){
$next_title = Title::newFromText( $links[1][$i+1] );
if(!$next_title) return $err;
- $next_url = $next_title->getFullURL();
+ $next_url = $next_title->escapeFullURL();
}
else $next_url = '';
This is incorrect. The URLs in question are destined for JavaScript, not
plain HTML, so you need to use Xml::escapeJsString() or
Xml::encodeJsVar(), not htmlspecialchars(). Additionally, escaping
should always be done as close to the output as possible, to make
security review easier and mistakes less likely. wfPRNavigation() should
return the unescaped URLs, and its caller should escape them according
to what kind of output it is doing.
-- Tim Starling