Skip to content

Commit

Permalink
Disable reading XML files and URLs when handling user data.
Browse files Browse the repository at this point in the history
Allowing users to load arbitrary files/URLs with Xml is not desirable
when handing user input.
  • Loading branch information
markstory committed May 27, 2015
1 parent dddc504 commit 995d8d2
Show file tree
Hide file tree
Showing 3 changed files with 29 additions and 3 deletions.
2 changes: 1 addition & 1 deletion lib/Cake/Controller/Component/RequestHandlerComponent.php
Expand Up @@ -229,7 +229,7 @@ public function startup(Controller $controller) {
*/
public function convertXml($xml) {
try {
$xml = Xml::build($xml);
$xml = Xml::build($xml, ['readFile' => false]);
if (isset($xml->data)) {
return Xml::toArray($xml->data);
}
Expand Down
22 changes: 22 additions & 0 deletions lib/Cake/Test/Case/Utility/XmlTest.php
Expand Up @@ -167,6 +167,28 @@ public function testBuild() {
$this->assertNotRegExp('/encoding/', $obj->saveXML());
}

/**
* Test that the readFile option disables local file parsing.
*
* @expectedException XmlException
* @return void
*/
public function testBuildFromFileWhenDisabled() {
$xml = CAKE . 'Test' . DS . 'Fixture' . DS . 'sample.xml';
$obj = Xml::build($xml, ['readFile' => false]);
}

/**
* Test that the readFile option disables local file parsing.
*
* @expectedException XmlException
* @return void
*/
public function testBuildFromUrlWhenDisabled() {
$xml = 'http://www.google.com';
$obj = Xml::build($xml, ['readFile' => false]);
}

/**
* data provider function for testBuildInvalidData
*
Expand Down
8 changes: 6 additions & 2 deletions lib/Cake/Utility/Xml.php
Expand Up @@ -77,6 +77,9 @@ class Xml {
* - `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.
* - `readFile` Set to false to disable file reading. This is important to disable when
* putting user data into Xml::build(). If enabled local & remote files will be read if they exist.
* Defaults to true for backwards compatibility reasons.
* - If using array as input, you can pass `options` from Xml::fromArray.
*
* @param string|array $input XML string, a path to a file, a URL or an array
Expand All @@ -91,16 +94,17 @@ public static function build($input, $options = array()) {
$defaults = array(
'return' => 'simplexml',
'loadEntities' => false,
'readFile' => true
);
$options += $defaults;

if (is_array($input) || is_object($input)) {
return self::fromArray((array)$input, $options);
} elseif (strpos($input, '<') !== false) {
return self::_loadXml($input, $options);
} elseif (file_exists($input)) {
} elseif ($options['readFile'] && file_exists($input)) {
return self::_loadXml(file_get_contents($input), $options);
} elseif (strpos($input, 'http://') === 0 || strpos($input, 'https://') === 0) {
} elseif ($options['readFile'] && strpos($input, 'http://') === 0 || strpos($input, 'https://') === 0) {
try {
$socket = new HttpSocket(array('request' => array('redirect' => 10)));
$response = $socket->get($input);
Expand Down

0 comments on commit 995d8d2

Please sign in to comment.