Skip to content
This repository was archived by the owner on May 16, 2018. It is now read-only.

Commit 281a325

Browse files
author
matthew
committed
[ZF-12293] Ensure XML-RPC response does not load entities
Merges r24975 - Disable loading external entities - Does not affect actual internal functionality, but could be used as a potential local DoS on clients git-svn-id: http://framework.zend.com/svn/framework/standard/branches/release-1.12@24977 44c647ce-9c0f-0410-b52a-842ac1e357ba
1 parent 728636d commit 281a325

File tree

4 files changed

+32
-5
lines changed

4 files changed

+32
-5
lines changed

library/Zend/XmlRpc/Request.php

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -307,6 +307,7 @@ public function loadXml($request)
307307
$loadEntities = libxml_disable_entity_loader(true);
308308
try {
309309
$xml = new SimpleXMLElement($request);
310+
libxml_disable_entity_loader($loadEntities);
310311
} catch (Exception $e) {
311312
// Not valid XML
312313
$this->_fault = new Zend_XmlRpc_Fault(631);
@@ -320,7 +321,6 @@ public function loadXml($request)
320321
// Missing method name
321322
$this->_fault = new Zend_XmlRpc_Fault(632);
322323
$this->_fault->setEncoding($this->getEncoding());
323-
libxml_disable_entity_loader($loadEntities);
324324
return false;
325325
}
326326

@@ -334,7 +334,6 @@ public function loadXml($request)
334334
if (!isset($param->value)) {
335335
$this->_fault = new Zend_XmlRpc_Fault(633);
336336
$this->_fault->setEncoding($this->getEncoding());
337-
libxml_disable_entity_loader($loadEntities);
338337
return false;
339338
}
340339

@@ -345,7 +344,6 @@ public function loadXml($request)
345344
} catch (Exception $e) {
346345
$this->_fault = new Zend_XmlRpc_Fault(636);
347346
$this->_fault->setEncoding($this->getEncoding());
348-
libxml_disable_entity_loader($loadEntities);
349347
return false;
350348
}
351349
}
@@ -354,7 +352,6 @@ public function loadXml($request)
354352
$this->_params = $argv;
355353
}
356354

357-
libxml_disable_entity_loader($loadEntities);
358355
$this->_xml = $request;
359356

360357
return true;

library/Zend/XmlRpc/Response.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,11 +176,15 @@ public function loadXml($response)
176176
return false;
177177
}
178178

179+
// @see ZF-12293 - disable external entities for security purposes
180+
$loadEntities = libxml_disable_entity_loader(true);
181+
$useInternalXmlErrors = libxml_use_internal_errors(true);
179182
try {
180-
$useInternalXmlErrors = libxml_use_internal_errors(true);
181183
$xml = new SimpleXMLElement($response);
184+
libxml_disable_entity_loader($loadEntities);
182185
libxml_use_internal_errors($useInternalXmlErrors);
183186
} catch (Exception $e) {
187+
libxml_disable_entity_loader($loadEntities);
184188
libxml_use_internal_errors($useInternalXmlErrors);
185189
// Not valid XML
186190
$this->_fault = new Zend_XmlRpc_Fault(651);
@@ -205,6 +209,7 @@ public function loadXml($response)
205209

206210
try {
207211
if (!isset($xml->params) || !isset($xml->params->param) || !isset($xml->params->param->value)) {
212+
require_once 'Zend/XmlRpc/Value/Exception.php';
208213
throw new Zend_XmlRpc_Value_Exception('Missing XML-RPC value in XML');
209214
}
210215
$valueXml = $xml->params->param->value->asXML();

tests/Zend/XmlRpc/ResponseTest.php

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -252,4 +252,19 @@ public function trackError($error)
252252
{
253253
$this->_errorOccured = true;
254254
}
255+
256+
/**
257+
* @group ZF-12293
258+
*/
259+
public function testDoesNotAllowExternalEntities()
260+
{
261+
$payload = file_get_contents(dirname(__FILE__) . '/_files/ZF12293-response.xml');
262+
$payload = sprintf($payload, 'file://' . realpath(dirname(__FILE__) . '/_files/ZF12293-payload.txt'));
263+
$this->_response->loadXml($payload);
264+
$value = $this->_response->getReturnValue();
265+
$this->assertTrue(empty($value));
266+
if (is_string($value)) {
267+
$this->assertNotContains('Local file inclusion', $value);
268+
}
269+
}
255270
}
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?xml version="1.0"?>
2+
<!DOCTYPE foo [
3+
<!ELEMENT methodResponse ANY >
4+
<!ENTITY xxe SYSTEM "%s" >
5+
]>
6+
<methodResponse>
7+
<params>
8+
<param><value><string>&xxe;</string></value></param>
9+
</params>
10+
</methodResponse>

0 commit comments

Comments
 (0)