tonyair/silverstripe-framework
1
0
Fork 0
mirror of https://github.com/silverstripe/silverstripe-framework synced 2024-06-24 13:39:26 +02:00
Code Issues Projects Releases Wiki Activity

[ss-2015-021] Fix rewrite hash links XSS

Browse Source
This commit is contained in:
Damian Mooyman 2015-10-29 11:53:44 +13:00
parent b338efe5a7
commit 132e9b3e2f
4 changed files with 40 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

29
tests/view/SSViewerTest.php
View File

@ -2,6 +2,13 @@
class SSViewerTest extends SapphireTest {
/**
* Backup of $_SERVER global
*
* @var array
*/
protected $oldServer = array();
protected $extraDataObjects = array(
'SSViewerTest_Object',
);
@ -10,6 +17,12 @@ class SSViewerTest extends SapphireTest {
parent::setUp();
Config::inst()->update('SSViewer', 'source_file_comments', false);
Config::inst()->update('SSViewer_FromString', 'cache_template', false);
$this->oldServer = $_SERVER;
}
public function tearDown() {
$_SERVER = $this->oldServer;
parent::tearDown();
}
/**
@ -1156,10 +1169,13 @@ after')
$orig = Config::inst()->get('SSViewer', 'rewrite_hash_links');
Config::inst()->update('SSViewer', 'rewrite_hash_links', true);
$_SERVER['REQUEST_URI'] = 'http://path/to/file?foo"onclick="alert(\'xss\')""';
$_SERVER['HTTP_HOST'] = 'www.mysite.com';
$_SERVER['REQUEST_URI'] = '//file.com?foo"onclick="alert(\'xss\')""';
// Emulate SSViewer::process()
$base = Convert::raw2att($_SERVER['REQUEST_URI']);
// Note that leading double slashes have been rewritten to prevent these being mis-interepreted
// as protocol-less absolute urls
$base = Convert::raw2att('/file.com?foo"onclick="alert(\'xss\')""');
$tmplFile = TEMP_FOLDER . '/SSViewerTest_testRewriteHashlinks_' . sha1(rand()) . '.ss';
@ -1227,10 +1243,11 @@ after')
$obj = new ViewableData();
$obj->InsertedLink = '<a class="inserted" href="#anchor">InsertedLink</a>';
$result = $tmpl->process($obj);
$this->assertContains(
'<a class="inserted" href="<?php echo Convert::raw2att(',
$result
);
$code = <<<'EOC'
<a class="inserted" href="<?php echo Convert::raw2att(preg_replace("/^(\/)+/", "/", $_SERVER['REQUEST_URI'])); ?>#anchor">InsertedLink</a>
EOC;
$this->assertContains($code, $result);
// TODO Fix inline links in PHP mode
// $this->assertContains(
// '<a class="inline" href="<?php echo str_replace(',

10
view/SSTemplateParser.php
View File

@ -4681,11 +4681,15 @@ class SSTemplateParser extends Parser implements TemplateParser {
// TODO: This is pretty ugly & gets applied on all files not just html. I wonder if we can make this
// non-dynamically calculated
$code = <<<'EOC'
(\Config::inst()->get('SSViewer', 'rewrite_hash_links')
? \Convert::raw2att( preg_replace("/^(\\/)+/", "/", $_SERVER['REQUEST_URI'] ) )
: "")
EOC;
// Because preg_replace replacement requires escaped slashes, addcslashes here
$text = preg_replace(
'/(<a[^>]+href *= *)"#/i',
'\\1"\' . (Config::inst()->get(\'SSViewer\', \'rewrite_hash_links\') ?' .
' Convert::raw2att( $_SERVER[\'REQUEST_URI\'] ) : "") .
\'#',
'\\1"\' . ' . addcslashes($code, '\\') . ' . \'#',
$text
);

10
view/SSTemplateParser.php.inc
View File

@ -1135,11 +1135,15 @@ class SSTemplateParser extends Parser implements TemplateParser {
// TODO: This is pretty ugly & gets applied on all files not just html. I wonder if we can make this
// non-dynamically calculated
$code = <<<'EOC'
(\Config::inst()->get('SSViewer', 'rewrite_hash_links')
? \Convert::raw2att( preg_replace("/^(\\/)+/", "/", $_SERVER['REQUEST_URI'] ) )
: "")
EOC;
// Because preg_replace replacement requires escaped slashes, addcslashes here
$text = preg_replace(
'/(<a[^>]+href *= *)"#/i',
'\\1"\' . (Config::inst()->get(\'SSViewer\', \'rewrite_hash_links\') ?' .
' Convert::raw2att( $_SERVER[\'REQUEST_URI\'] ) : "") .
\'#',
'\\1"\' . ' . addcslashes($code, '\\') . ' . \'#',
$text
);

6
view/SSViewer.php
View File

@ -1129,10 +1129,10 @@ class SSViewer implements Flushable {
$rewrite = Config::inst()->get('SSViewer', 'rewrite_hash_links');
if($this->rewriteHashlinks && $rewrite) {
if(strpos($output, '<base') !== false) {
if($rewrite === 'php') {
$thisURLRelativeToBase = "<?php echo Convert::raw2att(\$_SERVER['REQUEST_URI']); ?>";
if($rewrite === 'php') {
$thisURLRelativeToBase = "<?php echo Convert::raw2att(preg_replace(\"/^(\\\\/)+/\", \"/\", \$_SERVER['REQUEST_URI'])); ?>";
} else {
$thisURLRelativeToBase = Convert::raw2att($_SERVER['REQUEST_URI']);
$thisURLRelativeToBase = Convert::raw2att(preg_replace("/^(\\/)+/", "/", $_SERVER['REQUEST_URI']));
}
$output = preg_replace('/(<a[^>]+href *= *)"#/i', '\\1"' . $thisURLRelativeToBase . '#', $output);