From 2390698ea9f77f60b42cce2c4f78aaa6f500f7f1 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Sun, 28 Oct 2018 21:39:14 +0000 Subject: [PATCH 1/3] FIX Replace Convert JSON methods with json_* methods, deprecated from SilverStripe 4.4 --- src/DataFormatter/JSONDataFormatter.php | 8 ++++---- tests/unit/RestfulServerTest.php | 8 ++++---- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/DataFormatter/JSONDataFormatter.php b/src/DataFormatter/JSONDataFormatter.php index efdfa2f..0ff743d 100644 --- a/src/DataFormatter/JSONDataFormatter.php +++ b/src/DataFormatter/JSONDataFormatter.php @@ -52,7 +52,7 @@ class JSONDataFormatter extends DataFormatter */ public function convertArray($array) { - return Convert::array2json($array); + return json_encode($array); } /** @@ -65,7 +65,7 @@ class JSONDataFormatter extends DataFormatter */ public function convertDataObject(DataObjectInterface $obj, $fields = null, $relations = null) { - return Convert::array2json($this->convertDataObjectToJSONObject($obj, $fields, $relations)); + return json_encode($this->convertDataObjectToJSONObject($obj, $fields, $relations)); } /** @@ -186,7 +186,7 @@ class JSONDataFormatter extends DataFormatter "items" => $items )); - return Convert::array2json($serobj); + return json_encode($serobj); } /** @@ -195,7 +195,7 @@ class JSONDataFormatter extends DataFormatter */ public function convertStringToArray($strData) { - return Convert::json2array($strData); + return json_decode($strData, true); } public static function cast(FieldType\DBField $dbfield) diff --git a/tests/unit/RestfulServerTest.php b/tests/unit/RestfulServerTest.php index 0e2695c..cd0268b 100644 --- a/tests/unit/RestfulServerTest.php +++ b/tests/unit/RestfulServerTest.php @@ -307,7 +307,7 @@ class RestfulServerTest extends SapphireTest 'Accept' => 'application/json' )); $this->assertEquals(202, $response->getStatusCode()); // Accepted - $obj = Convert::json2obj($response->getBody()); + $obj = json_decode($response->getBody()); $this->assertEquals($comment1->ID, $obj->ID); $this->assertEquals('updated', $obj->Comment); @@ -318,7 +318,7 @@ 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::json2obj($response->getBody()); + $obj = json_decode($response->getBody()); $this->assertEquals($comment1->ID, $obj->ID); $this->assertEquals('updated', $obj->Comment); @@ -368,7 +368,7 @@ class RestfulServerTest extends SapphireTest $headers = array('Accept' => 'application/json'); $response = Director::test($url, null, null, 'GET', null, $headers); $this->assertEquals(200, $response->getStatusCode()); // Success - $obj = Convert::json2obj($response->getBody()); + $obj = json_decode($response->getBody()); $this->assertEquals($comment1->ID, $obj->ID); $this->assertEquals('application/json', $response->getHeader('Content-Type')); } @@ -659,7 +659,7 @@ class RestfulServerTest extends SapphireTest $response = Director::test($url, null, null, 'GET'); $this->assertEquals(200, $response->getStatusCode()); $this->assertNotContains('Unspeakable', $response->getBody()); - $responseArray = Convert::json2array($response->getBody()); + $responseArray = json_decode($response->getBody(), true); $this->assertSame(0, $responseArray['totalSize']); // With authentication From dbb8e18644bb11647c65cea91f4c1978497864f0 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Mon, 20 May 2019 11:04:52 +1200 Subject: [PATCH 2/3] [CVE-2019-12149] Fixed potential SQL injection vulnerability in RestfulServer --- src/RestfulServer.php | 36 +++++++++------- tests/unit/RestfulServerTest.php | 45 +++++++++++++++++++- tests/unit/RestfulServerTest.yml | 4 +- tests/unit/Stubs/RestfulServerTestAuthor.php | 2 +- 4 files changed, 68 insertions(+), 19 deletions(-) diff --git a/src/RestfulServer.php b/src/RestfulServer.php index 38da66f..360f041 100644 --- a/src/RestfulServer.php +++ b/src/RestfulServer.php @@ -2,20 +2,20 @@ namespace SilverStripe\RestfulServer; -use SilverStripe\ORM\ArrayList; -use SilverStripe\Core\Config\Config; +use SilverStripe\CMS\Model\SiteTree; use SilverStripe\Control\Controller; -use SilverStripe\ORM\DataList; -use SilverStripe\ORM\DataObject; use SilverStripe\Control\Director; use SilverStripe\Control\HTTPRequest; +use SilverStripe\Core\Config\Config; +use SilverStripe\Core\Injector\Injector; +use SilverStripe\ORM\ArrayList; +use SilverStripe\ORM\DataList; +use SilverStripe\ORM\DataObject; use SilverStripe\ORM\SS_List; use SilverStripe\ORM\ValidationException; use SilverStripe\ORM\ValidationResult; use SilverStripe\Security\Member; use SilverStripe\Security\Security; -use SilverStripe\CMS\Model\SiteTree; -use SilverStripe\Core\Injector\Injector; /** * Generic RESTful server, which handles webservice access to arbitrary DataObjects. @@ -205,23 +205,29 @@ class RestfulServer extends Controller * @todo Access checking * * @param string $className - * @param Int $id + * @param int $id * @param string $relation * @return string The serialized representation of the requested object(s) - usually XML or JSON. */ protected function getHandler($className, $id, $relationName) { - $sort = ''; + $sort = ['ID' => 'ASC']; - if ($this->request->getVar('sort')) { - $dir = $this->request->getVar('dir'); - $sort = array($this->request->getVar('sort') => ($dir ? $dir : 'ASC')); + if ($sortQuery = $this->request->getVar('sort')) { + /** @var DataObject $singleton */ + $singleton = singleton($className); + // Only apply a sort filter if it is a valid field on the DataObject + if ($singleton && $singleton->hasDatabaseField($sortQuery)) { + $sort = [ + $sortQuery => $this->request->getVar('dir') === 'DESC' ? 'DESC' : 'ASC', + ]; + } } - $limit = array( - 'start' => $this->request->getVar('start'), - 'limit' => $this->request->getVar('limit') - ); + $limit = [ + 'start' => (int) $this->request->getVar('start'), + 'limit' => (int) $this->request->getVar('limit'), + ]; $params = $this->request->getVars(); diff --git a/tests/unit/RestfulServerTest.php b/tests/unit/RestfulServerTest.php index c188f35..0ad7684 100644 --- a/tests/unit/RestfulServerTest.php +++ b/tests/unit/RestfulServerTest.php @@ -49,7 +49,7 @@ class RestfulServerTest extends SapphireTest { parent::setUp(); Director::config()->set('alternate_base_url', $this->baseURI); - Security::setCurrentUser(null); + $this->logOut(); } public function testApiAccess() @@ -613,6 +613,49 @@ class RestfulServerTest extends SapphireTest ); } + public function testGetWithSortDescending() + { + $urlSafeClassname = $this->urlSafeClassname(RestfulServerTestAuthor::class); + $url = "{$this->baseURI}/api/v1/{$urlSafeClassname}?sort=FirstName&dir=DESC&fields=FirstName"; + + $response = Director::test($url); + $results = Convert::xml2array($response->getBody()); + + $this->assertSame('Author 4', $results[$urlSafeClassname][0]['FirstName']); + $this->assertSame('Author 3', $results[$urlSafeClassname][1]['FirstName']); + $this->assertSame('Author 2', $results[$urlSafeClassname][2]['FirstName']); + $this->assertSame('Author 1', $results[$urlSafeClassname][3]['FirstName']); + } + + public function testGetWithSortAscending() + { + $urlSafeClassname = $this->urlSafeClassname(RestfulServerTestAuthor::class); + $url = "{$this->baseURI}/api/v1/{$urlSafeClassname}?sort=FirstName&dir=ASC&fields=FirstName"; + + $response = Director::test($url); + $results = Convert::xml2array($response->getBody()); + + $this->assertSame('Author 1', $results[$urlSafeClassname][0]['FirstName']); + $this->assertSame('Author 2', $results[$urlSafeClassname][1]['FirstName']); + $this->assertSame('Author 3', $results[$urlSafeClassname][2]['FirstName']); + $this->assertSame('Author 4', $results[$urlSafeClassname][3]['FirstName']); + } + + public function testGetSortsByIdWhenInvalidSortColumnIsProvided() + { + $urlSafeClassname = $this->urlSafeClassname(RestfulServerTestAuthor::class); + $url = "{$this->baseURI}/api/v1/{$urlSafeClassname}?sort=Surname&dir=DESC&fields=FirstName"; + + $response = Director::test($url); + + $results = Convert::xml2array($response->getBody()); + + $this->assertSame('Author 1', $results[$urlSafeClassname][0]['FirstName']); + $this->assertSame('Author 2', $results[$urlSafeClassname][1]['FirstName']); + $this->assertSame('Author 3', $results[$urlSafeClassname][2]['FirstName']); + $this->assertSame('Author 4', $results[$urlSafeClassname][3]['FirstName']); + } + public function testApiAccessWithPOST() { $urlSafeClassname = $this->urlSafeClassname(RestfulServerTestAuthorRating::class); diff --git a/tests/unit/RestfulServerTest.yml b/tests/unit/RestfulServerTest.yml index 63dd21e..6bdc16a 100644 --- a/tests/unit/RestfulServerTest.yml +++ b/tests/unit/RestfulServerTest.yml @@ -46,10 +46,10 @@ SilverStripe\RestfulServer\Tests\Stubs\RestfulServerTestAuthor: author2: FirstName: Author 2 author3: - Firstname: Author 3 + FirstName: Author 3 author4: FirstName: Author 4 - RelatedAuthors: + RelatedAuthors: - =>SilverStripe\RestfulServer\Tests\Stubs\RestfulServerTestAuthor.author2 - =>SilverStripe\RestfulServer\Tests\Stubs\RestfulServerTestAuthor.author3 SilverStripe\RestfulServer\Tests\Stubs\RestfulServerTestAuthorRating: diff --git a/tests/unit/Stubs/RestfulServerTestAuthor.php b/tests/unit/Stubs/RestfulServerTestAuthor.php index 0145f03..faeff8b 100644 --- a/tests/unit/Stubs/RestfulServerTestAuthor.php +++ b/tests/unit/Stubs/RestfulServerTestAuthor.php @@ -12,7 +12,7 @@ class RestfulServerTestAuthor extends DataObject implements TestOnly private static $table_name = 'RestfulServerTestAuthor'; private static $db = array( - 'Name' => 'Text', + 'FirstName' => 'Text', ); private static $many_many = array( From 5505f938750786faabb2f7cc80109c9747681691 Mon Sep 17 00:00:00 2001 From: Robbie Averill Date: Tue, 11 Jun 2019 14:09:07 +1200 Subject: [PATCH 3/3] Use trusty dist for Travis builds --- .travis.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.travis.yml b/.travis.yml index c07a6b7..92d9338 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,5 +1,7 @@ language: php +dist: trusty + env: global: - COMPOSER_ROOT_VERSION=2.0.x-dev