[ZF-12293] Disable loading external XML entities

- Disables loading of external XML entities, thus preventing local file inclusion git-svn-id: http://framework.zend.com/svn/framework/standard/branches/release-1.12@24973 44c647ce-9c0f-0410-b52a-842ac1e357ba
zendframework · Jun 18, 2012 · 728636d · 728636d
1 parent e8d4705
commit 728636d
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 0 deletions.
diff --git a/library/Zend/XmlRpc/Request.php b/library/Zend/XmlRpc/Request.php
@@ -303,12 +303,15 @@ public function loadXml($request)
             return false;
         }
 
+        // @see ZF-12293 - disable external entities for security purposes
+        $loadEntities = libxml_disable_entity_loader(true);
         try {
             $xml = new SimpleXMLElement($request);
         } catch (Exception $e) {
             // Not valid XML
             $this->_fault = new Zend_XmlRpc_Fault(631);
             $this->_fault->setEncoding($this->getEncoding());
+            libxml_disable_entity_loader($loadEntities);
             return false;
         }
 
@@ -317,6 +320,7 @@ public function loadXml($request)
             // Missing method name
             $this->_fault = new Zend_XmlRpc_Fault(632);
             $this->_fault->setEncoding($this->getEncoding());
+            libxml_disable_entity_loader($loadEntities);
             return false;
         }
 
@@ -330,6 +334,7 @@ public function loadXml($request)
                 if (!isset($param->value)) {
                     $this->_fault = new Zend_XmlRpc_Fault(633);
                     $this->_fault->setEncoding($this->getEncoding());
+                    libxml_disable_entity_loader($loadEntities);
                     return false;
                 }
 
@@ -340,6 +345,7 @@ public function loadXml($request)
                 } catch (Exception $e) {
                     $this->_fault = new Zend_XmlRpc_Fault(636);
                     $this->_fault->setEncoding($this->getEncoding());
+                    libxml_disable_entity_loader($loadEntities);
                     return false;
                 }
             }
@@ -348,6 +354,7 @@ public function loadXml($request)
             $this->_params = $argv;
         }
 
+        libxml_disable_entity_loader($loadEntities);
         $this->_xml = $request;
 
         return true;

diff --git a/tests/Zend/XmlRpc/RequestTest.php b/tests/Zend/XmlRpc/RequestTest.php
@@ -349,4 +349,19 @@ public function testSetGetEncoding()
         $this->assertEquals('ISO-8859-1', $this->_request->getEncoding());
         $this->assertEquals('ISO-8859-1', Zend_XmlRpc_Value::getGenerator()->getEncoding());
     }
+
+    /**
+     * @group ZF-12293
+     */
+    public function testDoesNotAllowExternalEntities()
+    {
+        $payload = file_get_contents(dirname(__FILE__) . '/_files/ZF12293-request.xml');
+        $payload = sprintf($payload, 'file://' . realpath(dirname(__FILE__) . '/_files/ZF12293-payload.txt'));
+        $this->_request->loadXml($payload);
+        $method = $this->_request->getMethod();
+        $this->assertTrue(empty($method));
+        if (is_string($method)) {
+            $this->assertNotContains('Local file inclusion', $method);
+        }
+    }
 }
diff --git a/tests/Zend/XmlRpc/_files/ZF12293-payload.txt b/tests/Zend/XmlRpc/_files/ZF12293-payload.txt
@@ -0,0 +1 @@
+Local file inclusion
diff --git a/tests/Zend/XmlRpc/_files/ZF12293-request.xml b/tests/Zend/XmlRpc/_files/ZF12293-request.xml
@@ -0,0 +1,8 @@
+<?xml version="1.0"?>
+<!DOCTYPE foo [
+<!ELEMENT methodName ANY >
+<!ENTITY xxe SYSTEM "%s" >
+]>
+<methodCall>
+    <methodName>&xxe;</methodName>
+</methodCall>