diff --git a/_config/dev.yml b/_config/dev.yml
index ca74c082f..4c1636bc4 100644
--- a/_config/dev.yml
+++ b/_config/dev.yml
@@ -17,3 +17,6 @@ SilverStripe\Dev\DevelopmentAdmin:
controller: Silverstripe\Dev\DevConfigController
links:
config: 'View the current config, useful for debugging'
+
+SilverStripe\Dev\CSSContentParser:
+ disable_xml_external_entities: true
diff --git a/docs/en/04_Changelogs/4.8.0.md b/docs/en/04_Changelogs/4.8.0.md
index 4791153e1..611cd7714 100644
--- a/docs/en/04_Changelogs/4.8.0.md
+++ b/docs/en/04_Changelogs/4.8.0.md
@@ -1,5 +1,18 @@
# 4.8.0 (Unreleased)
+## Security patches
+
+This release contains security patches. Some of those patches might require some
+updates to your project.
+
+* [CVE-2020-25817 XXE Vulnerability in CSSContentParser](https://www.silverstripe.org/download/security-releases/CVE-2020-25817)
+
+### CVE-2020-25817 XXE Vulnerability in CSSContentParser {#CVE-2020-25817}
+
+A tool intended for dev-only use CSSContentParser parses HTML using a the SimpleXML parser. Older versions
+of libxml do not have external entity loading disabled by default. This security patches explicitly
+disables external entity loading. It can be re-enabled if required via [configuration](/_config/dev.yml)
+
## Overview
- [Support for silverstripe/graphql v4](#graphql-v4)
diff --git a/src/Dev/CSSContentParser.php b/src/Dev/CSSContentParser.php
index 6074ad81a..c28a1c0d2 100644
--- a/src/Dev/CSSContentParser.php
+++ b/src/Dev/CSSContentParser.php
@@ -2,6 +2,7 @@
namespace SilverStripe\Dev;
+use SilverStripe\Core\Config\Configurable;
use SilverStripe\Core\Injector\Injectable;
use SimpleXMLElement;
use tidy;
@@ -25,6 +26,7 @@ use Exception;
class CSSContentParser
{
use Injectable;
+ use Configurable;
protected $simpleXML = null;
@@ -56,6 +58,13 @@ class CSSContentParser
$tidy = $content;
}
+ // Prevent loading of external entities to prevent XXE attacks
+ // Note: as of libxml 2.9.0 entity substitution is disabled by default so this won't be required
+ if ($this->config()->get('disable_xml_external_entities')) {
+ libxml_set_external_entity_loader(function () {
+ return null;
+ });
+ }
$this->simpleXML = @simplexml_load_string($tidy, 'SimpleXMLElement', LIBXML_NOWARNING);
if (!$this->simpleXML) {
throw new Exception('CSSContentParser::__construct(): Could not parse content.'
diff --git a/tests/php/Dev/CSSContentParserTest.php b/tests/php/Dev/CSSContentParserTest.php
index 5377e7865..a9551abac 100644
--- a/tests/php/Dev/CSSContentParserTest.php
+++ b/tests/php/Dev/CSSContentParserTest.php
@@ -50,4 +50,14 @@ HTML
$result = $parser->getBySelector('#A .other');
$this->assertEquals("result", $result[0] . '');
}
+
+ public function testXmlEntitiesDisabled()
+ {
+ // CSSContentParser uses simplexml to parse html
+ // Ensure XML entities are not substituted in to prevent XXE attacks
+ $xml = ']>Hello &myentity;
';
+ $parser = new CSSContentParser($xml);
+ $div = $parser->getBySelector('div')[0]->asXML();
+ $this->assertEquals('Hello &myentity;
', $div);
+ }
}