From 0ee07bc62e32ddde2680a48fe13fd58c28a208aa Mon Sep 17 00:00:00 2001 From: Enrico Zimuel Date: Wed, 26 Feb 2014 16:02:02 +0100 Subject: [PATCH 1/2] Fix for potential XXE/XEE attacks on XML --- src/Fault.php | 9 +++++---- src/Response.php | 25 ++++--------------------- test/FaultTest.php | 2 +- 3 files changed, 10 insertions(+), 26 deletions(-) diff --git a/src/Fault.php b/src/Fault.php index 4a38659..9af84a2 100644 --- a/src/Fault.php +++ b/src/Fault.php @@ -10,6 +10,7 @@ namespace Zend\XmlRpc; use SimpleXMLElement; +use Zend\Xml\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 (\Zend\Xml\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 index be2c4f6..241df1a 100644 --- a/src/Response.php +++ b/src/Response.php @@ -9,6 +9,8 @@ namespace Zend\XmlRpc; +use Zend\Xml\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 (\Zend\Xml\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 index 1361c29..f55f3a8 100644 --- 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'); } From 7a42486b63797a37af5c26be1bd3d4fb235a5939 Mon Sep 17 00:00:00 2001 From: Enrico Zimuel Date: Fri, 28 Feb 2014 16:47:52 +0100 Subject: [PATCH 2/2] Use external ZendXml\Security --- src/Fault.php | 4 ++-- src/Response.php | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Fault.php b/src/Fault.php index 9af84a2..bb18801 100644 --- a/src/Fault.php +++ b/src/Fault.php @@ -10,7 +10,7 @@ namespace Zend\XmlRpc; use SimpleXMLElement; -use Zend\Xml\Security as XmlSecurity; +use ZendXml\Security as XmlSecurity; /** * XMLRPC Faults @@ -182,7 +182,7 @@ public function loadXml($fault) $xmlErrorsFlag = libxml_use_internal_errors(true); try { $xml = XmlSecurity::scan($fault); - } catch (\Zend\Xml\Exception\RuntimeException $e) { + } catch (\ZendXml\Exception\RuntimeException $e) { // Unsecure XML throw new Exception\RuntimeException('Failed to parse XML fault: ' . $e->getMessage(), 500, $e); } diff --git a/src/Response.php b/src/Response.php index 241df1a..f853758 100644 --- a/src/Response.php +++ b/src/Response.php @@ -9,7 +9,7 @@ namespace Zend\XmlRpc; -use Zend\Xml\Security as XmlSecurity; +use ZendXml\Security as XmlSecurity; /** * XmlRpc Response @@ -155,7 +155,7 @@ public function loadXml($response) try { $xml = XmlSecurity::scan($response); - } catch (\Zend\Xml\Exception\RuntimeException $e) { + } catch (\ZendXml\Exception\RuntimeException $e) { $this->fault = new Fault(651); $this->fault->setEncoding($this->getEncoding()); return false;