aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSteve Clay <steve@mrclay.org>2013-07-11 13:24:01 -0400
committerPaweł Sroka <srokap@gmail.com>2013-11-04 03:34:21 +0100
commitd53447f7e6b3277f3249d9a70e56ec01a90c3a60 (patch)
treea61fa62cef82fef01254849bbbd70dbf149e854a
parent550ef1fe32fc8da940c42359f7a6347e65138c85 (diff)
downloadelgg-d53447f7e6b3277f3249d9a70e56ec01a90c3a60.tar.gz
elgg-d53447f7e6b3277f3249d9a70e56ec01a90c3a60.tar.bz2
Disable loading external entities during XML parsing
-rw-r--r--engine/classes/ElggAutoP.php14
-rw-r--r--engine/classes/ElggXMLElement.php8
-rw-r--r--engine/tests/regression/trac_bugs.php10
-rw-r--r--engine/tests/test_files/xxe/external_entity.txt1
-rw-r--r--engine/tests/test_files/xxe/request.xml8
5 files changed, 39 insertions, 2 deletions
diff --git a/engine/classes/ElggAutoP.php b/engine/classes/ElggAutoP.php
index 71536c433..05842d1b2 100644
--- a/engine/classes/ElggAutoP.php
+++ b/engine/classes/ElggAutoP.php
@@ -110,12 +110,19 @@ class ElggAutoP {
// http://www.php.net/manual/en/domdocument.loadhtml.php#95463
libxml_use_internal_errors(true);
+ // Do not load entities. May be unnecessary, better safe than sorry
+ $disable_load_entities = libxml_disable_entity_loader(true);
+
if (!$this->_doc->loadHTML("<html><meta http-equiv='content-type' "
. "content='text/html; charset={$this->encoding}'><body>{$html}</body>"
. "</html>")) {
+
+ libxml_disable_entity_loader($disable_load_entities);
return false;
}
+ libxml_disable_entity_loader($disable_load_entities);
+
$this->_xpath = new DOMXPath($this->_doc);
// start processing recursively at the BODY element
$nodeList = $this->_xpath->query('//body[1]');
@@ -135,9 +142,16 @@ class ElggAutoP {
// re-parse so we can handle new AUTOP elements
+ // Do not load entities. May be unnecessary, better safe than sorry
+ $disable_load_entities = libxml_disable_entity_loader(true);
+
if (!$this->_doc->loadHTML($html)) {
+ libxml_disable_entity_loader($disable_load_entities);
return false;
}
+
+ libxml_disable_entity_loader($disable_load_entities);
+
// must re-create XPath object after DOM load
$this->_xpath = new DOMXPath($this->_doc);
diff --git a/engine/classes/ElggXMLElement.php b/engine/classes/ElggXMLElement.php
index 6f2633e25..cbd3fc5ce 100644
--- a/engine/classes/ElggXMLElement.php
+++ b/engine/classes/ElggXMLElement.php
@@ -20,7 +20,12 @@ class ElggXMLElement {
if ($xml instanceof SimpleXMLElement) {
$this->_element = $xml;
} else {
+ // do not load entities
+ $disable_load_entities = libxml_disable_entity_loader(true);
+
$this->_element = new SimpleXMLElement($xml);
+
+ libxml_disable_entity_loader($disable_load_entities);
}
}
@@ -123,5 +128,4 @@ class ElggXMLElement {
}
return false;
}
-
-} \ No newline at end of file
+}
diff --git a/engine/tests/regression/trac_bugs.php b/engine/tests/regression/trac_bugs.php
index ef1348cf6..e6773c8af 100644
--- a/engine/tests/regression/trac_bugs.php
+++ b/engine/tests/regression/trac_bugs.php
@@ -373,4 +373,14 @@ class ElggCoreRegressionBugsTest extends ElggCoreUnitTest {
//delete group and annotations
$group->delete();
}
+
+ public function test_ElggXMLElement_does_not_load_external_entities() {
+ $payload = file_get_contents(dirname(dirname(__FILE__)) . '/test_files/xxe/request.xml');
+ $payload = sprintf($payload, 'file://' . realpath(dirname(dirname(__FILE__)) . '/test_files/xxe/external_entity.txt'));
+
+ $el = new ElggXMLElement($payload);
+ $chidren = $el->getChildren();
+ $content = $chidren[0]->getContent();
+ $this->assertNoPattern('/secret/', $content);
+ }
}
diff --git a/engine/tests/test_files/xxe/external_entity.txt b/engine/tests/test_files/xxe/external_entity.txt
new file mode 100644
index 000000000..536aca34d
--- /dev/null
+++ b/engine/tests/test_files/xxe/external_entity.txt
@@ -0,0 +1 @@
+secret \ No newline at end of file
diff --git a/engine/tests/test_files/xxe/request.xml b/engine/tests/test_files/xxe/request.xml
new file mode 100644
index 000000000..4390f9db2
--- /dev/null
+++ b/engine/tests/test_files/xxe/request.xml
@@ -0,0 +1,8 @@
+<?xml version="1.0"?>
+<!DOCTYPE foo [
+<!ELEMENT methodName ANY >
+<!ENTITY xxe SYSTEM "%s" >
+]>
+<methodCall>
+ <methodName>test&xxe;test</methodName>
+</methodCall>