]> gitweb.fluxo.info Git - lorea/elgg.git/commitdiff
projects / lorea / elgg.git / commitdiff
? search:
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: 550ef1f)
Disable loading external entities during XML parsing
authorSteve Clay <steve@mrclay.org>
Thu, 11 Jul 2013 17:24:01 +0000 (13:24 -0400)
committerPaweł Sroka <srokap@gmail.com>
Mon, 4 Nov 2013 02:34:21 +0000 (03:34 +0100)
engine/classes/ElggAutoP.php patch | blob | history
engine/classes/ElggXMLElement.php patch | blob | history
engine/tests/regression/trac_bugs.php patch | blob | history
engine/tests/test_files/xxe/external_entity.txt [new file with mode: 0644] patch | blob
engine/tests/test_files/xxe/request.xml [new file with mode: 0644] patch | blob

diff --git a/engine/classes/ElggAutoP.php b/engine/classes/ElggAutoP.php
index 71536c433e89d7fa1af37e3d4dc064e6b4c002b3..05842d1b25d8b2ba2fe39821d89797e33f4f369b 100644 (file)
--- 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 6f2633e25147a55ea015dc8bf0aaf257eee991c1..cbd3fc5cedd3bbc3ad9156053b0ee67100700762 100644 (file)
--- 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 ef1348cf60813f6adc4238c63bfaae28ab88263b..e6773c8af6ff67bfad213d5bfa3115ddce8b25c0 100644 (file)
--- 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 (file)
index 0000000..536aca3
--- /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 (file)
index 0000000..4390f9d
--- /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>
Social Networking Engine