From b175798fabfe9b935be0d8219f9235a0d13ea65f Mon Sep 17 00:00:00 2001 From: terry Date: Wed, 3 Jun 2020 15:24:30 +0200 Subject: [PATCH 1/6] array_key_exists() on objects is deprecated --- model/ArrayList.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/model/ArrayList.php b/model/ArrayList.php index 8b6b07bcb..cb4ec3c38 100644 --- a/model/ArrayList.php +++ b/model/ArrayList.php @@ -475,7 +475,7 @@ class ArrayList extends ViewableData implements SS_List, SS_Filterable, SS_Sorta return false; } - return array_key_exists($by, $firstRecord); + return is_array($firstRecord) ? array_key_exists($by, $firstRecord) : property_exists($by, $firstRecord); } /** From d3b23e7024add23de1cb643a44e30d249c2b7cd6 Mon Sep 17 00:00:00 2001 From: Maxime Rainville Date: Thu, 23 Apr 2020 16:41:04 +1200 Subject: [PATCH 2/6] [CVE-2020-9311] Escape First Name when displaying re-login screen --- security/CMSSecurity.php | 2 +- security/MemberLoginForm.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/security/CMSSecurity.php b/security/CMSSecurity.php index 69f6e9fc5..2fb1c97f5 100644 --- a/security/CMSSecurity.php +++ b/security/CMSSecurity.php @@ -86,7 +86,7 @@ class CMSSecurity extends Security { 'CMSSecurity.TimedOutTitleMember', 'Hey {name}!
Your session has timed out.', 'Title for CMS popup login form for a known user', - array('name' => $member->FirstName) + array('name' => Convert::raw2xml($member->FirstName)) ); } else { return _t( diff --git a/security/MemberLoginForm.php b/security/MemberLoginForm.php index ded8cfc9b..b9bb8ce67 100644 --- a/security/MemberLoginForm.php +++ b/security/MemberLoginForm.php @@ -139,7 +139,7 @@ JS; $this->message = _t( 'Member.LOGGEDINAS', "You're logged in as {name}.", - array('name' => $member->{$this->loggedInAsField}) + array('name' => Convert::raw2xml($member->{$this->loggedInAsField})) ); } From 98926e4e6c26d1d43bb1faf516d15bdb2739556e Mon Sep 17 00:00:00 2001 From: Maxime Rainville Date: Tue, 28 Apr 2020 16:29:41 +1200 Subject: [PATCH 3/6] [CVE-2019-19326] Stop honouring X-HTTP-Method-Override header, X-Original-Url header and _method POST variable. Add SS_HTTPRequest::setHttpMethod(). --- control/Director.php | 4 +- control/HTTPRequest.php | 43 +++++++++----- main.php | 5 -- tests/FakeController.php | 7 +-- tests/control/HTTPRequestTest.php | 97 +++++++++++++++++++++++++------ 5 files changed, 110 insertions(+), 46 deletions(-) diff --git a/control/Director.php b/control/Director.php index 3f62ecc39..f5a10a9c4 100644 --- a/control/Director.php +++ b/control/Director.php @@ -117,9 +117,7 @@ class Director implements TemplateGlobalProvider { } $req = new SS_HTTPRequest( - (isset($_SERVER['X-HTTP-Method-Override'])) - ? $_SERVER['X-HTTP-Method-Override'] - : $_SERVER['REQUEST_METHOD'], + $_SERVER['REQUEST_METHOD'], $url, $_GET, ArrayLib::array_merge_recursive((array) $_POST, (array) $_FILES), diff --git a/control/HTTPRequest.php b/control/HTTPRequest.php index a5eefc4ce..536ad2353 100644 --- a/control/HTTPRequest.php +++ b/control/HTTPRequest.php @@ -11,9 +11,6 @@ * match() to get the information that they need out of the URL. This is generally handled by * {@link RequestHandler::handleRequest()}. * - * @todo Accept X_HTTP_METHOD_OVERRIDE http header and $_REQUEST['_method'] to override request types (useful for - * webclients not supporting PUT and DELETE) - * * @package framework * @subpackage control */ @@ -106,7 +103,7 @@ class SS_HTTPRequest implements ArrayAccess { * Construct a SS_HTTPRequest from a URL relative to the site root. */ public function __construct($httpMethod, $url, $getVars = array(), $postVars = array(), $body = null) { - $this->httpMethod = strtoupper(self::detect_method($httpMethod, $postVars)); + $this->httpMethod = strtoupper($httpMethod); $this->setUrl($url); $this->getVars = (array) $getVars; @@ -726,24 +723,40 @@ class SS_HTTPRequest implements ArrayAccess { } /** - * Gets the "real" HTTP method for a request. - * - * Used to work around browser limitations of form - * submissions to GET and POST, by overriding the HTTP method - * with a POST parameter called "_method" for PUT, DELETE, HEAD. - * Using GET for the "_method" override is not supported, - * as GET should never carry out state changes. - * Alternatively you can use a custom HTTP header 'X-HTTP-Method-Override' - * to override the original method in {@link Director::direct()}. - * The '_method' POST parameter overrules the custom HTTP header. + * Explicitly set the HTTP method for this request. + * @param string $method + * @return $this + */ + public function setHttpMethod($method) { + if(!self::isValidHttpMethod($method)) { + user_error('SS_HTTPRequest::setHttpMethod: Invalid HTTP method', E_USER_ERROR); + } + + $this->httpMethod = strtoupper($method); + return $this; + } + + /** + * @param string $method + * @return bool + */ + private static function isValidHttpMethod($method) { + return in_array(strtoupper($method), array('GET','POST','PUT','DELETE','HEAD')); + } + + /** + * Gets the "real" HTTP method for a request. This method is no longer used to mitigate the risk of web cache + * poisoning. * + * @see https://www.silverstripe.org/download/security-releases/CVE-2019-19326 * @param string $origMethod Original HTTP method from the browser request * @param array $postVars * @return string HTTP method (all uppercase) + * @deprecated 3.7.5 */ public static function detect_method($origMethod, $postVars) { if(isset($postVars['_method'])) { - if(!in_array(strtoupper($postVars['_method']), array('GET','POST','PUT','DELETE','HEAD'))) { + if (!self::isValidHttpMethod($postVars['_method'])) { user_error('Director::direct(): Invalid "_method" parameter', E_USER_ERROR); } return strtoupper($postVars['_method']); diff --git a/main.php b/main.php index 4a56ca1e0..e0f2fc04e 100644 --- a/main.php +++ b/main.php @@ -60,11 +60,6 @@ require_once('core/Constants.php'); // we handle our own cache headers in this application session_cache_limiter(''); -// IIS will sometimes generate this. -if(!empty($_SERVER['HTTP_X_ORIGINAL_URL'])) { - $_SERVER['REQUEST_URI'] = $_SERVER['HTTP_X_ORIGINAL_URL']; -} - // Enable the entity loader to be able to load XML in Zend_Locale_Data libxml_disable_entity_loader(false); diff --git a/tests/FakeController.php b/tests/FakeController.php index b54af6a6e..4489cd471 100644 --- a/tests/FakeController.php +++ b/tests/FakeController.php @@ -10,12 +10,7 @@ class FakeController extends Controller { $this->pushCurrent(); - $request = new SS_HTTPRequest( - (isset($_SERVER['X-HTTP-Method-Override'])) - ? $_SERVER['X-HTTP-Method-Override'] - : $_SERVER['REQUEST_METHOD'], - '/' - ); + $request = new SS_HTTPRequest($_SERVER['REQUEST_METHOD'], '/'); $this->setRequest($request); $this->setResponse(new SS_HTTPResponse()); diff --git a/tests/control/HTTPRequestTest.php b/tests/control/HTTPRequestTest.php index e1b1fc3ca..58dfb999f 100644 --- a/tests/control/HTTPRequestTest.php +++ b/tests/control/HTTPRequestTest.php @@ -51,8 +51,12 @@ class HTTPRequestTest extends SapphireTest { array('_method' => 'DELETE') ); $this->assertTrue( + $request->isPOST(), + '_method override is no longer honored.' + ); + $this->assertFalse( $request->isDELETE(), - 'POST with valid method override to DELETE' + 'DELETE _method override is not honored.' ); $request = new SS_HTTPRequest( @@ -61,9 +65,9 @@ class HTTPRequestTest extends SapphireTest { array(), array('_method' => 'put') ); - $this->assertTrue( + $this->assertFalse( $request->isPUT(), - 'POST with valid method override to PUT' + 'PUT _method override is not honored.' ); $request = new SS_HTTPRequest( @@ -72,33 +76,92 @@ class HTTPRequestTest extends SapphireTest { array(), array('_method' => 'head') ); - $this->assertTrue( + $this->assertFalse( $request->isHEAD(), - 'POST with valid method override to HEAD ' + 'HEAD _method override is not honored.' ); $request = new SS_HTTPRequest( 'POST', 'admin/crm', - array(), - array('_method' => 'head') + array('_method' => 'delete') ); - $this->assertTrue( - $request->isHEAD(), - 'POST with valid method override to HEAD' + $this->assertFalse( + $request->isDELETE(), + 'DELETE _method override is not honored.' ); + } - $request = new SS_HTTPRequest( - 'POST', - 'admin/crm', - array('_method' => 'head') + public function detectMethodDataProvider() + { + return array( + 'Plain POST request' => array('POST', array(), 'POST'), + 'Plain GET request' => array('GET', array(), 'GET'), + 'Plain DELETE request' => array('DELETE', array(), 'DELETE'), + 'Plain PUT request' => array('PUT', array(), 'PUT'), + 'Plain HEAD request' => array('HEAD', array(), 'HEAD'), + + 'Request with GET method override' => array('POST', array('_method' => 'GET'), 'GET'), + 'Request with HEAD method override' => array('POST', array('_method' => 'HEAD'), 'HEAD'), + 'Request with DELETE method override' => array('POST', array('_method' => 'DELETE'), 'DELETE'), + 'Request with PUT method override' => array('POST', array('_method' => 'PUT'), 'PUT'), + 'Request with POST method override' => array('POST', array('_method' => 'POST'), 'POST'), + + 'Request with mixed case method override' => array('POST', array('_method' => 'gEt'), 'GET'), ); - $this->assertTrue( - $request->isPOST(), - 'POST with invalid method override by GET parameters to HEAD' + } + + + /** + * @dataProvider detectMethodDataProvider + */ + public function testDetectMethod($realMethod, $post, $expected) + { + $actual = SS_HTTPRequest::detect_method($realMethod, $post); + $this->assertEquals($expected, $actual); + } + + + /** + * @expectedException PHPUnit_Framework_Error + */ + public function testBadDetectMethod() + { + SS_HTTPRequest::detect_method('POST', array('_method' => 'Boom')); + } + + public function setHttpMethodDataProvider() + { + return array( + 'POST request' => array('POST','POST'), + 'GET request' => array('GET', 'GET'), + 'DELETE request' => array('DELETE', 'DELETE'), + 'PUT request' => array('PUT', 'PUT'), + 'HEAD request' => array('HEAD', 'HEAD'), + 'Mixed case POST' => array('gEt', 'GET'), ); } + /** + * @dataProvider setHttpMethodDataProvider + */ + public function testSetHttpMethod($method, $expected) + { + $request = new SS_HTTPRequest('GET', '/hello'); + $returnedRequest = $request->setHttpMethod($method); + $this->assertEquals($expected, $request->httpMethod()); + $this->assertEquals($request, $returnedRequest); + } + + /** + * @expectedException PHPUnit_Framework_Error + */ + public function testBadSetHttpMethod() + { + $request = new SS_HTTPRequest('GET', '/hello'); + $request->setHttpMethod('boom'); + } + public function testRequestVars() { $getVars = array( 'first' => 'a', From 074b28cf937821a0d5627d3f19836ede1d662395 Mon Sep 17 00:00:00 2001 From: Maxime Rainville Date: Mon, 4 May 2020 21:11:34 +1200 Subject: [PATCH 4/6] [CVE-2019-19326] Add changelog for CVE-2019-19326 --- docs/en/04_Changelogs/3.7.4.md | 4 +- docs/en/04_Changelogs/3.7.5.md | 68 ++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 2 deletions(-) create mode 100644 docs/en/04_Changelogs/3.7.5.md diff --git a/docs/en/04_Changelogs/3.7.4.md b/docs/en/04_Changelogs/3.7.4.md index 11d60454b..3d7954cce 100644 --- a/docs/en/04_Changelogs/3.7.4.md +++ b/docs/en/04_Changelogs/3.7.4.md @@ -1,4 +1,4 @@ -# 3.7.4 (unreleased) +# 3.7.4 * [Minor update to support PHP 7.4](https://github.com/silverstripe/silverstripe-framework/pull/9110) @@ -38,4 +38,4 @@ This fork is not a supported module and SilverStripe does not commit to maintain * 2019-02-25 [adbc560bd](https://github.com/silverstripe/silverstripe-framework/commit/adbc560bd70ba2e071f94a41a084768819196ee7) Address PR feedback. (Maxime Rainville) * 2019-02-21 [4ec1a682c](https://github.com/silverstripe/silverstripe-framework/commit/4ec1a682cf354e2425ef4fd6598c7de8e807bcc7) Renable the ability to do dynamic assignment with DBField (Maxime Rainville) * 2019-02-19 [ab5f09a9f](https://github.com/silverstripe/silverstripe-framework/commit/ab5f09a9f3ec12333c748dd68bfc504b5e509bfc) Updated unit test were targeting Float/Int which don't exist on PHP7 (#8810) (Maxime Rainville) - \ No newline at end of file + diff --git a/docs/en/04_Changelogs/3.7.5.md b/docs/en/04_Changelogs/3.7.5.md new file mode 100644 index 000000000..e2f601c85 --- /dev/null +++ b/docs/en/04_Changelogs/3.7.5.md @@ -0,0 +1,68 @@ +# 3.7.5 + +* [CVE-2019-19326 Web Cache Poisoning](#CVE-2019-19326) + +## CVE-2019-19326 Web Cache Poisoning {#CVE-2019-19326} + +Silverstripe sites using HTTP cache headers and HTTP caching proxies (e.g. CDNs) can be susceptible to web cache poisoning through the: +* `X-Original-Url` HTTP header +* `X-HTTP-Method-Override` HTTP header +* `_method` POST variable. + +In order to remedy this vulnerability, Silverstripe Framework 3.7.5 removes native support for these features. While this is technically a semantic versioning breakage, these features are inherently insecure and date back to a time when browsers didn't natively support the full range of HTTP methods. Sites who still require these features will have highly unusual requirements that are best served by a tailored solution. + +### Re-enabling the support for removed features + +These features are best implemented by defining a `RequestFilter`. Request Filters are similar to the more modern concept of "middleware" as defined by the PSR-15 standard and supported by Silverstripe 4. + +The following example illustrate how to implement a `RequestFilter` that restore support for the `X-Original-Url` header and the `_method` POST parameter for request originating from a trusted proxy. + +```php +getHeader('X-Original-Url'); + if ($originalUrl) { + $request->setUrl($originalUrl); + $_SERVER['REQUEST_URI'] = $originalUrl; + } + + $methodOverride = $request->postVar('_method'); + $validMethods = ['GET', 'POST', 'PUT', 'DELETE', 'HEAD']; + if ($methodOverride && in_array(strtoupper($methodOverride), $validMethods)) { + $request->setMethod($methodOverride); + } + } + + return true; + } + + public function postRequest(SS_HTTPRequest $request, SS_HTTPResponse $response, DataModel $model) + { + return true; + } +} +``` + +To learn more about re-implementing support for the disabled features: +* read [How to implement a Request Filter](/developer_guides/controllers/requestfilters) on the Silverstripe documentation +* read [how to configure trusted proxies](/developer_guides/security/secure_coding/#request-hostname-forgery) on the Silverstripe documentation +* review [api:RequestFilter] interface + +To learn more about middleware: +* read the [PSR-15: HTTP Server Request Handlers](https://www.php-fig.org/psr/psr-15/) standard +* read the [Silverstripe 4 documentation about HTTP Middlewares](https://docs.silverstripe.org/en/4/developer_guides/controllers/middlewares/) standard. + + + + From c96e9d2fe5e0fbea1da4059264e4da269889f55d Mon Sep 17 00:00:00 2001 From: Maxime Rainville Date: Thu, 9 Jul 2020 22:26:40 +1200 Subject: [PATCH 5/6] [CVE-2020-9311] Add public disclosure statement to changelog --- docs/en/04_Changelogs/3.7.5.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/docs/en/04_Changelogs/3.7.5.md b/docs/en/04_Changelogs/3.7.5.md index e2f601c85..7458de641 100644 --- a/docs/en/04_Changelogs/3.7.5.md +++ b/docs/en/04_Changelogs/3.7.5.md @@ -1,6 +1,7 @@ # 3.7.5 * [CVE-2019-19326 Web Cache Poisoning](#CVE-2019-19326) +* [CVE-2020-9311 Malicious user profile information can cause login form XSS](#CVE-2020-9311) ## CVE-2019-19326 Web Cache Poisoning {#CVE-2019-19326} @@ -63,6 +64,15 @@ To learn more about middleware: * read the [PSR-15: HTTP Server Request Handlers](https://www.php-fig.org/psr/psr-15/) standard * read the [Silverstripe 4 documentation about HTTP Middlewares](https://docs.silverstripe.org/en/4/developer_guides/controllers/middlewares/) standard. +[Review the CVE-2019-19326 public disclosure](https://www.silverstripe.org/download/security-releases/cve-2019-19326) + +## CVE-2020-9311 Malicious user profile information can cause login form XSS {#CVE-2020-9311} + +Malicious users with a valid Silverstripe login (usually CMS access) can craft profile information which can lead to XSS for other users through specially crafted login form URLs. + +[Review the CVE-2020-9311 public disclosure](https://www.silverstripe.org/download/security-releases/cve-2020-9311) + + From f2b8946407b7f44d09dd3bbccadb3db4e30357f7 Mon Sep 17 00:00:00 2001 From: Maxime Rainville Date: Tue, 14 Jul 2020 13:39:39 +1200 Subject: [PATCH 6/6] Added 3.7.5 changelog --- docs/en/04_Changelogs/3.7.5.md | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/docs/en/04_Changelogs/3.7.5.md b/docs/en/04_Changelogs/3.7.5.md index 7458de641..9b759a015 100644 --- a/docs/en/04_Changelogs/3.7.5.md +++ b/docs/en/04_Changelogs/3.7.5.md @@ -75,4 +75,21 @@ Malicious users with a valid Silverstripe login (usually CMS access) can craft p +## Change Log + +### Security + + * 2020-07-09 [c96e9d2fe](https://github.com/silverstripe/silverstripe-framework/commit/c96e9d2fe5e0fbea1da4059264e4da269889f55d) Add public disclosure statement to changelog (Maxime Rainville) - See [cve-2020-9311](https://www.silverstripe.org/download/security-releases/cve-2020-9311) + * 2020-05-04 [074b28cf9](https://github.com/silverstripe/silverstripe-framework/commit/074b28cf937821a0d5627d3f19836ede1d662395) Add changelog for CVE-2019-19326 (Maxime Rainville) - See [cve-2019-19326](https://www.silverstripe.org/download/security-releases/cve-2019-19326) + * 2020-04-28 [98926e4e6](https://github.com/silverstripe/silverstripe-framework/commit/98926e4e6c26d1d43bb1faf516d15bdb2739556e) Stop honouring X-HTTP-Method-Override header, X-Original-Url header and _method POST variable. Add SS_HTTPRequest::setHttpMethod(). (Maxime Rainville) - See [cve-2019-19326](https://www.silverstripe.org/download/security-releases/cve-2019-19326) + * 2020-04-23 [d3b23e702](https://github.com/silverstripe/silverstripe-framework/commit/d3b23e7024add23de1cb643a44e30d249c2b7cd6) Escape First Name when displaying re-login screen (Maxime Rainville) - See [cve-2020-9311](https://www.silverstripe.org/download/security-releases/cve-2020-9311) + +### Features and Enhancements + + * 2019-11-18 [54e7223d9](https://github.com/silverstripe/silverstripe-framework/commit/54e7223d981eee7f00244ad9a79187ee3f2f063a) Docs rebuild for compliance with Gatsby (#9316) (Aaron Carlino) + +### Bugfixes + + * 2020-04-01 [6c8dc0fd9](https://github.com/silverstripe/silverstripe-framework/commit/6c8dc0fd9957d0f497ccc3c700c0d805aff1269e) Fix deprecated php syntax (Dan Hensby) + * 2019-11-19 [42ab51230](https://github.com/silverstripe/silverstripe-framework/commit/42ab512306196d1010808adbe728f1fe179519aa) Fix broken callout tags (Aaron Carlino)