diff --git a/src/DataFormatter/JSONDataFormatter.php b/src/DataFormatter/JSONDataFormatter.php index 561a47b..a913c32 100644 --- a/src/DataFormatter/JSONDataFormatter.php +++ b/src/DataFormatter/JSONDataFormatter.php @@ -97,7 +97,7 @@ class JSONDataFormatter extends DataFormatter if ($this->relationDepth > 0) { foreach ($obj->hasOne() as $relName => $relClass) { - if (!singleton($relClass)->stat('api_access')) { + if (!$relClass::config()->get('api_access')) { continue; } @@ -132,7 +132,7 @@ class JSONDataFormatter extends DataFormatter $parts = explode('.', $relClass ?? ''); $relClass = array_shift($parts); - if (!singleton($relClass)->stat('api_access')) { + if (!$relClass::config()->get('api_access')) { continue; } diff --git a/src/DataFormatter/XMLDataFormatter.php b/src/DataFormatter/XMLDataFormatter.php index 42be941..45aa669 100644 --- a/src/DataFormatter/XMLDataFormatter.php +++ b/src/DataFormatter/XMLDataFormatter.php @@ -2,6 +2,7 @@ namespace SilverStripe\RestfulServer\DataFormatter; +use SimpleXMLElement; use SilverStripe\Control\Controller; use SilverStripe\Core\Convert; use SilverStripe\Dev\Debug; @@ -11,6 +12,7 @@ use SilverStripe\ORM\DataObjectInterface; use SilverStripe\Control\Director; use SilverStripe\ORM\SS_List; use SilverStripe\RestfulServer\RestfulServer; +use InvalidArgumentException; /** * Formats a DataObject's member fields into an XML string @@ -145,7 +147,7 @@ class XMLDataFormatter extends DataFormatter if ($this->relationDepth > 0) { foreach ($obj->hasOne() as $relName => $relClass) { - if (!singleton($relClass)->stat('api_access')) { + if (!singleton($relClass)::config()->get('api_access')) { continue; } @@ -171,7 +173,7 @@ class XMLDataFormatter extends DataFormatter //remove dot notation from relation names $parts = explode('.', $relClass ?? ''); $relClass = array_shift($parts); - if (!singleton($relClass)->stat('api_access')) { + if (!singleton($relClass)::config()->get('api_access')) { continue; } // backslashes in FQCNs kills both URIs and XML @@ -202,7 +204,7 @@ class XMLDataFormatter extends DataFormatter //remove dot notation from relation names $parts = explode('.', $relClass ?? ''); $relClass = array_shift($parts); - if (!singleton($relClass)->stat('api_access')) { + if (!singleton($relClass)::config()->get('api_access')) { continue; } // backslashes in FQCNs kills both URIs and XML @@ -261,6 +263,79 @@ class XMLDataFormatter extends DataFormatter */ public function convertStringToArray($strData) { - return Convert::xml2array($strData); + return self::xml2array($strData); + } + + /** + * This was copied from Convert::xml2array() which is deprecated/removed + * + * Converts an XML string to a PHP array + * See http://phpsecurity.readthedocs.org/en/latest/Injection-Attacks.html#xml-external-entity-injection + * + * @uses recursiveXMLToArray() + * @param string $val + * @param boolean $disableDoctypes Disables the use of DOCTYPE, and will trigger an error if encountered. + * false by default. + * @param boolean $disableExternals Does nothing because xml entities are removed + * @return array + * @throws Exception + */ + private static function xml2array($val, $disableDoctypes = false, $disableExternals = false) + { + // Check doctype + if ($disableDoctypes && strpos($val ?? '', '/', '', $val ?? ''); + + // If there's still an present, then it would be the result of a maliciously + // crafted XML document e.g. ENTITY ext SYSTEM "http://evil.com"> + if (strpos($val ?? '', ' before it was removed + $xml = new SimpleXMLElement($val ?? ''); + return self::recursiveXMLToArray($xml); + } + + /** + * @param SimpleXMLElement $xml + * + * @return mixed + */ + private static function recursiveXMLToArray($xml) + { + $x = null; + if ($xml instanceof SimpleXMLElement) { + $attributes = $xml->attributes(); + foreach ($attributes as $k => $v) { + if ($v) { + $a[$k] = (string) $v; + } + } + $x = $xml; + $xml = get_object_vars($xml); + } + if (is_array($xml)) { + if (count($xml ?? []) === 0) { + return (string)$x; + } // for CDATA + $r = []; + foreach ($xml as $key => $value) { + $r[$key] = self::recursiveXMLToArray($value); + } + // Attributes + if (isset($a)) { + $r['@'] = $a; + } + return $r; + } + + return (string) $xml; } } diff --git a/src/RestfulServer.php b/src/RestfulServer.php index 873021f..1a14b55 100644 --- a/src/RestfulServer.php +++ b/src/RestfulServer.php @@ -289,6 +289,10 @@ class RestfulServer extends Controller 'limit' => (int) $this->request->getVar('limit'), ]; + if ($limit['limit'] === 0) { + $limit = null; + } + $params = $this->request->getVars(); $responseFormatter = $this->getResponseDataFormatter($className); diff --git a/tests/unit/RestfulServerTest.php b/tests/unit/RestfulServerTest.php index 8591f6d..654ee8d 100644 --- a/tests/unit/RestfulServerTest.php +++ b/tests/unit/RestfulServerTest.php @@ -20,6 +20,7 @@ use SilverStripe\Dev\SapphireTest; use SilverStripe\RestfulServer\DataFormatter\JSONDataFormatter; use Page; use SilverStripe\Core\Config\Config; +use SilverStripe\RestfulServer\DataFormatter\XMLDataFormatter; /** * @@ -126,7 +127,8 @@ class RestfulServerTest extends SapphireTest $urlSafeClassname = $this->urlSafeClassname(RestfulServerTestAuthorRating::class); $url = "{$this->baseURI}/api/v1/$urlSafeClassname/" . $rating1->ID; $response = Director::test($url, null, null, 'GET'); - $responseArr = Convert::xml2array($response->getBody()); + $formatter = new XMLDataFormatter(); + $responseArr = $formatter->convertStringToArray($response->getBody()); $this->assertEquals(3, $responseArr['rate']); } @@ -165,7 +167,8 @@ class RestfulServerTest extends SapphireTest $response = Director::test($url, null, null, 'GET'); $this->assertEquals(200, $response->getStatusCode()); - $responseArr = Convert::xml2array($response->getBody()); + $formatter = new XMLDataFormatter(); + $responseArr = $formatter->convertStringToArray($response->getBody()); $xmlTagSafeClassName = $this->urlSafeClassname(RestfulServerTestAuthorRating::class); $ratingsArr = $responseArr['Ratings'][$xmlTagSafeClassName]; $this->assertEquals(2, count($ratingsArr ?? [])); @@ -192,7 +195,8 @@ class RestfulServerTest extends SapphireTest $response = Director::test($url, null, null, 'GET'); $this->assertEquals(200, $response->getStatusCode()); - $responseArr = Convert::xml2array($response->getBody()); + $formatter = new XMLDataFormatter(); + $responseArr = $formatter->convertStringToArray($response->getBody()); $xmlTagSafeClassName = $this->urlSafeClassname(RestfulServerTestAuthorRating::class); $this->assertTrue(array_key_exists('Ratings', $responseArr ?? [])); @@ -210,7 +214,9 @@ class RestfulServerTest extends SapphireTest $url = "{$this->baseURI}/api/v1/$urlSafeClassname/" . $author4->ID . '/RelatedAuthors'; $response = Director::test($url, null, null, 'GET'); $this->assertEquals(200, $response->getStatusCode()); - $arr = Convert::xml2array($response->getBody()); + + $formatter = new XMLDataFormatter(); + $arr = $formatter->convertStringToArray($response->getBody()); $xmlSafeClassName = $this->urlSafeClassname(RestfulServerTestAuthor::class); $authorsArr = $arr[$xmlSafeClassName]; @@ -239,7 +245,8 @@ class RestfulServerTest extends SapphireTest $response = Director::test($url, null, null, 'PUT', $body, $headers); $this->assertEquals(202, $response->getStatusCode()); // Accepted // Assumption: XML is default output - $responseArr = Convert::xml2array($response->getBody()); + $formatter = new XMLDataFormatter(); + $responseArr = $formatter->convertStringToArray($response->getBody()); $this->assertEquals($comment1->ID, $responseArr['ID']); $this->assertEquals('updated', $responseArr['Comment']); $this->assertEquals('Updated Comment', $responseArr['Name']); @@ -264,7 +271,8 @@ class RestfulServerTest extends SapphireTest $response = Director::test($url, null, null, 'POST', $body, $headers); $this->assertEquals(201, $response->getStatusCode()); // Created // Assumption: XML is default output - $responseArr = Convert::xml2array($response->getBody()); + $formatter = new XMLDataFormatter(); + $responseArr = $formatter->convertStringToArray($response->getBody()); $this->assertTrue($responseArr['ID'] > 0); $this->assertNotEquals($responseArr['ID'], $comment1->ID); $this->assertEquals('created', $responseArr['Comment']); @@ -339,7 +347,8 @@ class RestfulServerTest extends SapphireTest $body = 'updated'; $response = Director::test($url, null, null, 'PUT', $body, array('Content-Type'=>'text/xml')); $this->assertEquals(202, $response->getStatusCode()); // Accepted - $obj = Convert::xml2array($response->getBody()); + $formatter = new XMLDataFormatter(); + $obj = $formatter->convertStringToArray($response->getBody()); $this->assertEquals($comment1->ID, $obj['ID']); $this->assertEquals('updated', $obj['Comment']); @@ -350,7 +359,8 @@ class RestfulServerTest extends SapphireTest $response = Director::test($url, null, null, 'PUT', $body); $this->assertEquals(202, $response->getStatusCode()); // Accepted $this->assertEquals($url, $response->getHeader('Location')); - $obj = Convert::xml2array($response->getBody()); + $formatter = new XMLDataFormatter(); + $obj = $formatter->convertStringToArray($response->getBody()); $this->assertEquals($comment1->ID, $obj['ID']); $this->assertEquals('updated', $obj['Comment']); @@ -553,7 +563,8 @@ class RestfulServerTest extends SapphireTest ); $response = Director::test($url, $data, null, 'PUT'); // Assumption: XML is default output - $responseArr = Convert::xml2array($response->getBody()); + $formatter = new XMLDataFormatter(); + $responseArr = $formatter->convertStringToArray($response->getBody()); $this->assertEquals(42, $responseArr['Rating']); $this->assertNotEquals('haxx0red', $responseArr['WriteProtectedField']); } @@ -570,7 +581,8 @@ class RestfulServerTest extends SapphireTest ); $response = Director::test($url, $data, null, 'PUT'); // Assumption: XML is default output - $responseArr = Convert::xml2array($response->getBody()); + $formatter = new XMLDataFormatter(); + $responseArr = $formatter->convertStringToArray($response->getBody()); // should output with aliased name $this->assertEquals(42, $responseArr['rate']); } @@ -631,7 +643,8 @@ class RestfulServerTest extends SapphireTest $url = "{$this->baseURI}/api/v1/{$urlSafeClassname}?sort=FirstName&dir=DESC&fields=FirstName"; $response = Director::test($url); - $results = Convert::xml2array($response->getBody()); + $formatter = new XMLDataFormatter(); + $results = $formatter->convertStringToArray($response->getBody()); $this->assertSame('Author 4', $results[$urlSafeClassname][0]['FirstName']); $this->assertSame('Author 3', $results[$urlSafeClassname][1]['FirstName']); @@ -645,7 +658,8 @@ class RestfulServerTest extends SapphireTest $url = "{$this->baseURI}/api/v1/{$urlSafeClassname}?sort=FirstName&dir=ASC&fields=FirstName"; $response = Director::test($url); - $results = Convert::xml2array($response->getBody()); + $formatter = new XMLDataFormatter(); + $results = $formatter->convertStringToArray($response->getBody()); $this->assertSame('Author 1', $results[$urlSafeClassname][0]['FirstName']); $this->assertSame('Author 2', $results[$urlSafeClassname][1]['FirstName']); @@ -660,7 +674,8 @@ class RestfulServerTest extends SapphireTest $response = Director::test($url); - $results = Convert::xml2array($response->getBody()); + $formatter = new XMLDataFormatter(); + $results = $formatter->convertStringToArray($response->getBody()); $this->assertSame('Author 1', $results[$urlSafeClassname][0]['FirstName']); $this->assertSame('Author 2', $results[$urlSafeClassname][1]['FirstName']); @@ -678,7 +693,8 @@ class RestfulServerTest extends SapphireTest ]; $response = Director::test($url, $data, null, 'POST'); // Assumption: XML is default output - $responseArr = Convert::xml2array($response->getBody()); + $formatter = new XMLDataFormatter(); + $responseArr = $formatter->convertStringToArray($response->getBody()); $this->assertEquals(42, $responseArr['Rating']); $this->assertNotEquals('haxx0red', $responseArr['WriteProtectedField']); } @@ -692,7 +708,8 @@ class RestfulServerTest extends SapphireTest 'rate' => '42', ]; $response = Director::test($url, $data, null, 'POST'); - $responseArr = Convert::xml2array($response->getBody()); + $formatter = new XMLDataFormatter(); + $responseArr = $formatter->convertStringToArray($response->getBody()); $this->assertEquals(42, $responseArr['rate']); } @@ -722,7 +739,8 @@ class RestfulServerTest extends SapphireTest $this->assertEquals(200, $response->getStatusCode()); $this->assertStringContainsString('Unspeakable', $response->getBody()); // Assumption: default formatter is XML - $responseArray = Convert::xml2array($response->getBody()); + $formatter = new XMLDataFormatter(); + $responseArray = $formatter->convertStringToArray($response->getBody()); $this->assertEquals(1, $responseArray['@attributes']['totalSize']); unset($_SERVER['PHP_AUTH_USER']); unset($_SERVER['PHP_AUTH_PW']); @@ -737,7 +755,8 @@ class RestfulServerTest extends SapphireTest ]; $response = Director::test($url, $data, null, 'POST'); // Assumption: XML is default output - $responseArr = Convert::xml2array($response->getBody()); + $formatter = new XMLDataFormatter(); + $responseArr = $formatter->convertStringToArray($response->getBody()); $this->assertEquals('SilverStripe\\ORM\\ValidationException', $responseArr['type']); } @@ -750,7 +769,8 @@ class RestfulServerTest extends SapphireTest ]; $response = Director::test($url, $data, null, 'POST'); // Assumption: XML is default output - $responseArr = Convert::xml2array($response->getBody()); + $formatter = new XMLDataFormatter(); + $responseArr = $formatter->convertStringToArray($response->getBody()); $this->assertEquals(\Exception::class, $responseArr['type']); } diff --git a/tests/unit/XMLDataFormatterTest.php b/tests/unit/XMLDataFormatterTest.php new file mode 100644 index 0000000..8da70a1 --- /dev/null +++ b/tests/unit/XMLDataFormatterTest.php @@ -0,0 +1,77 @@ + + +]> + + My para + Ampersand & is retained and not double encoded + +XML + ; + $expected = [ + 'result' => [ + 'My para', + 'Ampersand & is retained and not double encoded' + ] + ]; + $formatter = new XMLDataFormatter(); + $actual = $formatter->convertStringToArray($inputXML); + $this->assertEquals($expected, $actual); + } + + /** + * Tests {@link Convert::xml2array()} if an exception the contains a reference to a removed + */ + public function testConvertStringToArrayEntityException() + { + $inputXML = << + + ]> + + Now include &long; lots of times to expand the in-memory size of this XML structure + &long;&long;&long; + + XML; + $this->expectException(Exception::class); + $this->expectExceptionMessage('String could not be parsed as XML'); + $formatter = new XMLDataFormatter(); + $formatter->convertStringToArray($inputXML); + } + + /** + * Tests {@link Convert::xml2array()} if an exception the contains a reference to a multiple removed + */ + public function testConvertStringToArrayMultipleEntitiesException() + { + $inputXML = << + ]> + + Now include &long; and &short; lots of times + &long;&long;&long;&short;&short;&short; + + XML; + $this->expectException(Exception::class); + $this->expectExceptionMessage('String could not be parsed as XML'); + $formatter = new XMLDataFormatter(); + $formatter->convertStringToArray($inputXML); + } +}