Merge branch 'master' of git://github.com/zendframework/zf2 into useS…

…tatementFixes
zendframework · Jun 20, 2012 · 17ed9ab · 17ed9ab
commit 17ed9ab
Show file tree

Hide file tree

Showing 8 changed files with 63 additions and 5 deletions.
diff --git a/src/Request.php b/src/Request.php
@@ -294,12 +294,16 @@ 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);
+            libxml_disable_entity_loader($loadEntities);
         } catch (\Exception $e) {
             // Not valid XML
             $this->_fault = new Fault(631);
             $this->_fault->setEncoding($this->getEncoding());
+            libxml_disable_entity_loader($loadEntities);
             return false;
         }
 

diff --git a/src/Response.php b/src/Response.php
@@ -167,11 +167,15 @@ 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 {
-            $useInternalXmlErrors = libxml_use_internal_errors(true);
             $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
             $this->_fault = new Fault(651);

diff --git a/src/Value/BigInteger.php b/src/Value/BigInteger.php
@@ -21,6 +21,8 @@
 
 namespace Zend\XmlRpc\Value;
 
+use Zend\Math\BigInteger\BigInteger as BigIntegerMath;
+
 /**
  * @category   Zend
  * @package    Zend_XmlRpc
@@ -35,15 +37,14 @@ class BigInteger extends Integer
      */
     public function __construct($value)
     {
-        $integer = new \Zend\Math\BigInteger();
-        $this->_value   = $integer->init($value);
-        $this->_type    = self::XMLRPC_TYPE_I8;
+        $this->_value = BigIntegerMath::factory()->init($value, 10);
+        $this->_type  = self::XMLRPC_TYPE_I8;
     }
 
     /**
      * Return bigint value object
      *
-     * @return \Zend\Math\BigInteger
+     * @return string
      */
     public function getValue()
     {

diff --git a/test/RequestTest.php b/test/RequestTest.php
@@ -333,4 +333,19 @@ public function testSetGetEncoding()
         $this->assertEquals('ISO-8859-1', $this->_request->getEncoding());
         $this->assertEquals('ISO-8859-1', 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/test/ResponseTest.php b/test/ResponseTest.php
@@ -259,4 +259,19 @@ public function trackError($error)
     {
         $this->_errorOccured = true;
     }
+
+    /**
+     * @group ZF-12293
+     */
+    public function testDoesNotAllowExternalEntities()
+    {
+        $payload = file_get_contents(dirname(__FILE__) . '/_files/ZF12293-response.xml');
+        $payload = sprintf($payload, 'file://' . realpath(dirname(__FILE__) . '/_files/ZF12293-payload.txt'));
+        $this->_response->loadXml($payload);
+        $value = $this->_response->getReturnValue();
+        $this->assertTrue(empty($value));
+        if (is_string($value)) {
+            $this->assertNotContains('Local file inclusion', $value);
+        }
+    }
 }
diff --git a/test/_files/ZF12293-payload.txt b/test/_files/ZF12293-payload.txt
@@ -0,0 +1 @@
+Local file inclusion
diff --git a/test/_files/ZF12293-request.xml b/test/_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>
diff --git a/test/_files/ZF12293-response.xml b/test/_files/ZF12293-response.xml
@@ -0,0 +1,10 @@
+<?xml version="1.0"?>
+<!DOCTYPE foo [
+<!ELEMENT methodResponse ANY >
+<!ENTITY xxe SYSTEM "%s" >
+]>
+<methodResponse>
+    <params>
+        <param><value><string>&xxe;</string></value></param>
+    </params>
+</methodResponse>