diff --git a/control/Director.php b/control/Director.php
index cc28ec8f2..7651ec2ef 100644
--- a/control/Director.php
+++ b/control/Director.php
@@ -796,14 +796,10 @@ class Director implements TemplateGlobalProvider {
* @param string $destURL - The URL to redirect to
*/
protected static function force_redirect($destURL) {
- $response = new SS_HTTPResponse(
- "
Your browser is not accepting header redirects
".
- "Please click here",
- 301
- );
+ $response = new SS_HTTPResponse();
+ $response->redirect($destURL, 301);
HTTP::add_cache_headers($response);
- $response->addHeader('Location', $destURL);
// TODO: Use an exception - ATM we can be called from _config.php, before Director#handleRequest's try block
$response->output();
diff --git a/core/Convert.php b/core/Convert.php
index 819d2abd0..89a4367a6 100644
--- a/core/Convert.php
+++ b/core/Convert.php
@@ -167,15 +167,32 @@ class Convert {
/**
* Converts an XML string to a PHP array
+ * See http://phpsecurity.readthedocs.org/en/latest/Injection-Attacks.html#xml-external-entity-injection
*
* @uses recursiveXMLToArray()
- * @param string
- *
+ * @param string $val
+ * @param boolean $disableDoctypes Disables the use of DOCTYPE, and will trigger an error if encountered.
+ * false by default.
+ * @param boolean $disableExternals Disables the loading of external entities. false by default.
* @return array
*/
- public static function xml2array($val) {
- $xml = new SimpleXMLElement($val);
- return self::recursiveXMLToArray($xml);
+ public static function xml2array($val, $disableDoctypes = false, $disableExternals = false) {
+ // Check doctype
+ if($disableDoctypes && preg_match('/\<\!DOCTYPE.+]\>/', $val)) {
+ throw new InvalidArgumentException('XML Doctype parsing disabled');
+ }
+
+ // Disable external entity loading
+ if($disableExternals) $oldVal = libxml_disable_entity_loader($disableExternals);
+ try {
+ $xml = new SimpleXMLElement($val);
+ $result = self::recursiveXMLToArray($xml);
+ } catch(Exception $ex) {
+ if($disableExternals) libxml_disable_entity_loader($oldVal);
+ throw $ex;
+ }
+ if($disableExternals) libxml_disable_entity_loader($oldVal);
+ return $result;
}
/**
diff --git a/tests/core/ConvertTest.php b/tests/core/ConvertTest.php
index 35141940d..baffd69c2 100644
--- a/tests/core/ConvertTest.php
+++ b/tests/core/ConvertTest.php
@@ -238,4 +238,55 @@ class ConvertTest extends SapphireTest {
Convert::raw2json($value)
);
}
+
+ public function testXML2Array() {
+ // Ensure an XML file at risk of entity expansion can be avoided safely
+ $inputXML = <<
+]>
+
+ Now include &long; lots of times to expand the in-memory size of this XML structure
+ &long;&long;&long;
+
+XML
+ ;
+ try {
+ Convert::xml2array($inputXML, true);
+ } catch(Exception $ex) {}
+ $this->assertTrue(
+ isset($ex)
+ && $ex instanceof InvalidArgumentException
+ && $ex->getMessage() === 'XML Doctype parsing disabled'
+ );
+
+ // Test without doctype validation
+ $expected = array(
+ 'result' => array(
+ "Now include SOME_SUPER_LONG_STRING lots of times to expand the in-memory size of this XML structure",
+ array(
+ 'long' => array(
+ array(
+ 'long' => 'SOME_SUPER_LONG_STRING'
+ ),
+ array(
+ 'long' => 'SOME_SUPER_LONG_STRING'
+ ),
+ array(
+ 'long' => 'SOME_SUPER_LONG_STRING'
+ )
+ )
+ )
+ )
+ );
+ $result = Convert::xml2array($inputXML, false, true);
+ $this->assertEquals(
+ $expected,
+ $result
+ );
+ $result = Convert::xml2array($inputXML, false, false);
+ $this->assertEquals(
+ $expected,
+ $result
+ );
+ }
}
diff --git a/tests/view/SSViewerTest.php b/tests/view/SSViewerTest.php
index d8349c50d..550eaa046 100644
--- a/tests/view/SSViewerTest.php
+++ b/tests/view/SSViewerTest.php
@@ -1131,8 +1131,10 @@ after')
public function testRewriteHashlinks() {
$orig = Config::inst()->get('SSViewer', 'rewrite_hash_links');
- Config::inst()->update('SSViewer', 'rewrite_hash_links', true);
-
+ Config::inst()->update('SSViewer', 'rewrite_hash_links', true);
+
+ $_SERVER['REQUEST_URI'] = 'http://path/to/file?foo"onclick="alert(\'xss\')""';
+
// Emulate SSViewer::process()
$base = Convert::raw2att($_SERVER['REQUEST_URI']);
@@ -1143,6 +1145,8 @@ after')
<% base_tag %>
+ ExternalInlineLink
+ $ExternalInsertedLink
InlineLink
$InsertedLink
@@ -1151,15 +1155,24 @@ after')
$tmpl = new SSViewer($tmplFile);
$obj = new ViewableData();
$obj->InsertedLink = 'InsertedLink';
+ $obj->ExternalInsertedLink = 'ExternalInsertedLink';
$result = $tmpl->process($obj);
$this->assertContains(
'InsertedLink',
$result
);
+ $this->assertContains(
+ 'ExternalInsertedLink',
+ $result
+ );
$this->assertContains(
'InlineLink',
$result
);
+ $this->assertContains(
+ 'ExternalInlineLink',
+ $result
+ );
$this->assertContains(
'',
$result,
@@ -1192,7 +1205,7 @@ after')
$obj->InsertedLink = 'InsertedLink';
$result = $tmpl->process($obj);
$this->assertContains(
- 'get(\'SSViewer\', \'rewrite_hash_links\') ?' .
- ' strip_tags( $_SERVER[\'REQUEST_URI\'] ) : "") .
+ ' Convert::raw2att( $_SERVER[\'REQUEST_URI\'] ) : "") .
\'#',
$text
);
diff --git a/view/SSTemplateParser.php.inc b/view/SSTemplateParser.php.inc
index 74fc27bbe..6bb7c550d 100644
--- a/view/SSTemplateParser.php.inc
+++ b/view/SSTemplateParser.php.inc
@@ -1138,7 +1138,7 @@ class SSTemplateParser extends Parser implements TemplateParser {
$text = preg_replace(
'/(]+href *= *)"#/i',
'\\1"\' . (Config::inst()->get(\'SSViewer\', \'rewrite_hash_links\') ?' .
- ' strip_tags( $_SERVER[\'REQUEST_URI\'] ) : "") .
+ ' Convert::raw2att( $_SERVER[\'REQUEST_URI\'] ) : "") .
\'#',
$text
);
diff --git a/view/SSViewer.php b/view/SSViewer.php
index 8a4ebf052..d5fdb68bb 100644
--- a/view/SSViewer.php
+++ b/view/SSViewer.php
@@ -1109,9 +1109,9 @@ class SSViewer implements Flushable {
if($this->rewriteHashlinks && $rewrite) {
if(strpos($output, '";
+ $thisURLRelativeToBase = "";
} else {
- $thisURLRelativeToBase = strip_tags($_SERVER['REQUEST_URI']);
+ $thisURLRelativeToBase = Convert::raw2att($_SERVER['REQUEST_URI']);
}
$output = preg_replace('/(]+href *= *)"#/i', '\\1"' . $thisURLRelativeToBase . '#', $output);