Browse files

Fix XML decoding attack via external entities.

  • Loading branch information...
1 parent 5fea194 commit 418672f524bc3aaf6257c1fadc816fe933116a1b @markstory markstory committed Jul 14, 2012
Showing with 51 additions and 13 deletions.
  1. +18 −0 lib/Cake/Test/Case/Utility/XmlTest.php
  2. +33 −13 lib/Cake/Utility/Xml.php
View
18 lib/Cake/Test/Case/Utility/XmlTest.php
@@ -1035,4 +1035,22 @@ public function testAmpInText() {
$this->assertContains('mark & mark', $result);
}
+/**
+ * Test that entity loading is disabled by default.
+ *
+ * @return void
+ */
+ public function testNoEntityLoading() {
+ $file = CAKE . 'VERSION.txt';
+ $xml = <<<XML
+<!DOCTYPE cakephp [
+ <!ENTITY payload SYSTEM "file://$file" >]>
+<request>
+ <xxe>&payload;</xxe>
+</request>
+XML;
+ $result = Xml::build($xml);
+ $this->assertEquals('', (string)$result->xxe);
+ }
+
}
View
46 lib/Cake/Utility/Xml.php
@@ -74,6 +74,8 @@ class Xml {
* ### Options
*
* - `return` Can be 'simplexml' to return object of SimpleXMLElement or 'domdocument' to return DOMDocument.
+ * - `loadEntities` Defaults to false. Set to true to enable loading of `<!ENTITY` definitions. This
+ * is disabled by default for security reasons.
* - If using array as input, you can pass `options` from Xml::fromArray.
*
* @param mixed $input XML string, a path to a file, an URL or an array
@@ -86,33 +88,51 @@ public static function build($input, $options = array()) {
$options = array('return' => (string)$options);
}
$defaults = array(
- 'return' => 'simplexml'
+ 'return' => 'simplexml',
+ 'loadEntities' => false,
);
$options = array_merge($defaults, $options);
if (is_array($input) || is_object($input)) {
return self::fromArray((array)$input, $options);
} elseif (strpos($input, '<') !== false) {
- if ($options['return'] === 'simplexml' || $options['return'] === 'simplexmlelement') {
- return new SimpleXMLElement($input, LIBXML_NOCDATA);
- }
- $dom = new DOMDocument();
- $dom->loadXML($input);
- return $dom;
+ return self::_loadXml($input, $options);
} elseif (file_exists($input) || strpos($input, 'http://') === 0 || strpos($input, 'https://') === 0) {
- if ($options['return'] === 'simplexml' || $options['return'] === 'simplexmlelement') {
- return new SimpleXMLElement($input, LIBXML_NOCDATA, true);
- }
- $dom = new DOMDocument();
- $dom->load($input);
- return $dom;
+ $input = file_get_contents($input);
+ return self::_loadXml($input, $options);
} elseif (!is_string($input)) {
throw new XmlException(__d('cake_dev', 'Invalid input.'));
}
throw new XmlException(__d('cake_dev', 'XML cannot be read.'));
}
/**
+ * Parse the input data and create either a SimpleXmlElement object or a DOMDocument.
+ *
+ * @param string $input The input to load.
+ * @param array $options The options to use. See Xml::build()
+ * @return SimpleXmlElement|DOMDocument.
+ */
+ protected static function _loadXml($input, $options) {
+ $hasDisable = function_exists('libxml_disable_entity_loader');
+ $internalErrors = libxml_use_internal_errors(true);
+ if ($hasDisable && !$options['loadEntities']) {
+ libxml_disable_entity_loader(true);
+ }
+ if ($options['return'] === 'simplexml' || $options['return'] === 'simplexmlelement') {
+ $xml = new SimpleXMLElement($input, LIBXML_NOCDATA);
+ } else {
+ $xml = new DOMDocument();
+ $xml->loadXML($input);
+ }
+ if ($hasDisable && !$options['loadEntities']) {
+ libxml_disable_entity_loader(false);
+ }
+ libxml_use_internal_errors($internalErrors);
+ return $xml;
+ }
+
+/**
* Transform an array into a SimpleXMLElement
*
* ### Options

0 comments on commit 418672f

Please sign in to comment.