Merge branch 'security/zf2014-01'

Resolves ZF2014-01 - XXE/XEE vulnerabilities
zendframework · Mar 6, 2014 · 204ccbe · 204ccbe
2 parents 43bb61e + 7a42486
commit 204ccbe
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 26 deletions.
diff --git a/src/Fault.php b/src/Fault.php
@@ -10,6 +10,7 @@
 namespace Zend\XmlRpc;
 
 use SimpleXMLElement;
+use ZendXml\Security as XmlSecurity;
 
 /**
  * XMLRPC Faults
@@ -180,10 +181,10 @@ public function loadXml($fault)
 
         $xmlErrorsFlag = libxml_use_internal_errors(true);
         try {
-            $xml = new SimpleXMLElement($fault);
-        } catch (\Exception $e) {
-            // Not valid XML
-            throw new Exception\InvalidArgumentException('Failed to parse XML fault: ' .  $e->getMessage(), 500, $e);
+            $xml = XmlSecurity::scan($fault);
+        } catch (\ZendXml\Exception\RuntimeException $e) {
+            // Unsecure XML
+            throw new Exception\RuntimeException('Failed to parse XML fault: ' .  $e->getMessage(), 500, $e);
         }
         if (!$xml instanceof SimpleXMLElement) {
             $errors = libxml_get_errors();

diff --git a/src/Response.php b/src/Response.php
@@ -9,6 +9,8 @@
 
 namespace Zend\XmlRpc;
 
+use ZendXml\Security as XmlSecurity;
+
 /**
  * XmlRpc Response
  *
@@ -151,28 +153,9 @@ public function loadXml($response)
             return false;
         }
 
-        // @see ZF-12293 - disable external entities for security purposes
-        $loadEntities         = libxml_disable_entity_loader(true);
-        $useInternalXmlErrors = libxml_use_internal_errors(true);
         try {
-            $dom = new \DOMDocument;
-            $dom->loadXML($response);
-            foreach ($dom->childNodes as $child) {
-                if ($child->nodeType === XML_DOCUMENT_TYPE_NODE) {
-                    throw new Exception\ValueException(
-                        'Invalid XML: Detected use of illegal DOCTYPE'
-                    );
-                }
-            }
-            // TODO: Locate why this passes tests but a simplexml import doesn't
-            //$xml = simplexml_import_dom($dom);
-            $xml = new \SimpleXMLElement($response);
-            libxml_disable_entity_loader($loadEntities);
-            libxml_use_internal_errors($useInternalXmlErrors);
-        } catch (\Exception $e) {
-            libxml_disable_entity_loader($loadEntities);
-            libxml_use_internal_errors($useInternalXmlErrors);
-            // Not valid XML
+            $xml = XmlSecurity::scan($response);
+        } catch (\ZendXml\Exception\RuntimeException $e) {
             $this->fault = new Fault(651);
             $this->fault->setEncoding($this->getEncoding());
             return false;

diff --git a/test/FaultTest.php b/test/FaultTest.php
@@ -146,7 +146,7 @@ public function testLoadXml()
 
     public function testLoadXmlThrowsExceptionOnInvalidInput()
     {
-        $this->setExpectedException('Zend\XmlRpc\Exception\InvalidArgumentException', 'Failed to parse XML fault: String could not be parsed as XML');
+        $this->setExpectedException('Zend\XmlRpc\Exception\InvalidArgumentException', 'Failed to parse XML fault');
         $parsed = $this->_fault->loadXml('foo');
     }