Merge branch '3.7' into 3

This commit is contained in:
Maxime Rainville 2020-07-15 10:37:23 +12:00
commit bc9e384fe1
10 changed files with 209 additions and 50 deletions

View File

@ -117,9 +117,7 @@ class Director implements TemplateGlobalProvider {
} }
$req = new SS_HTTPRequest( $req = new SS_HTTPRequest(
(isset($_SERVER['X-HTTP-Method-Override'])) $_SERVER['REQUEST_METHOD'],
? $_SERVER['X-HTTP-Method-Override']
: $_SERVER['REQUEST_METHOD'],
$url, $url,
$_GET, $_GET,
ArrayLib::array_merge_recursive((array) $_POST, (array) $_FILES), ArrayLib::array_merge_recursive((array) $_POST, (array) $_FILES),

View File

@ -11,9 +11,6 @@
* match() to get the information that they need out of the URL. This is generally handled by * match() to get the information that they need out of the URL. This is generally handled by
* {@link RequestHandler::handleRequest()}. * {@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 * @package framework
* @subpackage control * @subpackage control
*/ */
@ -106,7 +103,7 @@ class SS_HTTPRequest implements ArrayAccess {
* Construct a SS_HTTPRequest from a URL relative to the site root. * Construct a SS_HTTPRequest from a URL relative to the site root.
*/ */
public function __construct($httpMethod, $url, $getVars = array(), $postVars = array(), $body = null) { 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->setUrl($url);
$this->getVars = (array) $getVars; $this->getVars = (array) $getVars;
@ -726,24 +723,40 @@ class SS_HTTPRequest implements ArrayAccess {
} }
/** /**
* Gets the "real" HTTP method for a request. * Explicitly set the HTTP method for this request.
* * @param string $method
* Used to work around browser limitations of form * @return $this
* submissions to GET and POST, by overriding the HTTP method */
* with a POST parameter called "_method" for PUT, DELETE, HEAD. public function setHttpMethod($method) {
* Using GET for the "_method" override is not supported, if(!self::isValidHttpMethod($method)) {
* as GET should never carry out state changes. user_error('SS_HTTPRequest::setHttpMethod: Invalid HTTP method', E_USER_ERROR);
* 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. $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 string $origMethod Original HTTP method from the browser request
* @param array $postVars * @param array $postVars
* @return string HTTP method (all uppercase) * @return string HTTP method (all uppercase)
* @deprecated 3.7.5
*/ */
public static function detect_method($origMethod, $postVars) { public static function detect_method($origMethod, $postVars) {
if(isset($postVars['_method'])) { 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); user_error('Director::direct(): Invalid "_method" parameter', E_USER_ERROR);
} }
return strtoupper($postVars['_method']); return strtoupper($postVars['_method']);

View File

@ -0,0 +1,95 @@
# 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}
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
<?php
/**
* This is meant to illustrate how to implement a RequestFilter. It assumes your
* trusted proxy will strip the insecure data from any requests. If you blindly
* copy-paste this in in your code base, you'll simply replicate the vulnerability.
*/
class InsecureRequestProcessor implements RequestFilter
{
public function preRequest(SS_HTTPRequest $request, Session $session, DataModel $model)
{
if (TRUSTED_PROXY) {
$originalUrl = $request->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.
[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)
<!--- Changes below this line will be automatically regenerated -->
## 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)
<!--- Changes above this line will be automatically regenerated -->

View File

@ -60,11 +60,6 @@ require_once('core/Constants.php');
// we handle our own cache headers in this application // we handle our own cache headers in this application
session_cache_limiter(''); 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 // Enable the entity loader to be able to load XML in Zend_Locale_Data
libxml_disable_entity_loader(false); libxml_disable_entity_loader(false);

View File

@ -475,7 +475,7 @@ class ArrayList extends ViewableData implements SS_List, SS_Filterable, SS_Sorta
return false; return false;
} }
return array_key_exists($by, $firstRecord); return is_array($firstRecord) ? array_key_exists($by, $firstRecord) : property_exists($by, $firstRecord);
} }
/** /**

View File

@ -86,7 +86,7 @@ class CMSSecurity extends Security {
'CMSSecurity.TimedOutTitleMember', 'CMSSecurity.TimedOutTitleMember',
'Hey {name}!<br />Your session has timed out.', 'Hey {name}!<br />Your session has timed out.',
'Title for CMS popup login form for a known user', 'Title for CMS popup login form for a known user',
array('name' => $member->FirstName) array('name' => Convert::raw2xml($member->FirstName))
); );
} else { } else {
return _t( return _t(

View File

@ -139,7 +139,7 @@ JS;
$this->message = _t( $this->message = _t(
'Member.LOGGEDINAS', 'Member.LOGGEDINAS',
"You're logged in as {name}.", "You're logged in as {name}.",
array('name' => $member->{$this->loggedInAsField}) array('name' => Convert::raw2xml($member->{$this->loggedInAsField}))
); );
} }

View File

@ -10,12 +10,7 @@ class FakeController extends Controller {
$this->pushCurrent(); $this->pushCurrent();
$request = new SS_HTTPRequest( $request = new SS_HTTPRequest($_SERVER['REQUEST_METHOD'], '/');
(isset($_SERVER['X-HTTP-Method-Override']))
? $_SERVER['X-HTTP-Method-Override']
: $_SERVER['REQUEST_METHOD'],
'/'
);
$this->setRequest($request); $this->setRequest($request);
$this->setResponse(new SS_HTTPResponse()); $this->setResponse(new SS_HTTPResponse());

View File

@ -51,8 +51,12 @@ class HTTPRequestTest extends SapphireTest {
array('_method' => 'DELETE') array('_method' => 'DELETE')
); );
$this->assertTrue( $this->assertTrue(
$request->isPOST(),
'_method override is no longer honored.'
);
$this->assertFalse(
$request->isDELETE(), $request->isDELETE(),
'POST with valid method override to DELETE' 'DELETE _method override is not honored.'
); );
$request = new SS_HTTPRequest( $request = new SS_HTTPRequest(
@ -61,9 +65,9 @@ class HTTPRequestTest extends SapphireTest {
array(), array(),
array('_method' => 'put') array('_method' => 'put')
); );
$this->assertTrue( $this->assertFalse(
$request->isPUT(), $request->isPUT(),
'POST with valid method override to PUT' 'PUT _method override is not honored.'
); );
$request = new SS_HTTPRequest( $request = new SS_HTTPRequest(
@ -72,33 +76,92 @@ class HTTPRequestTest extends SapphireTest {
array(), array(),
array('_method' => 'head') array('_method' => 'head')
); );
$this->assertTrue( $this->assertFalse(
$request->isHEAD(), $request->isHEAD(),
'POST with valid method override to HEAD ' 'HEAD _method override is not honored.'
); );
$request = new SS_HTTPRequest( $request = new SS_HTTPRequest(
'POST', 'POST',
'admin/crm', 'admin/crm',
array(), array('_method' => 'delete')
array('_method' => 'head')
); );
$this->assertTrue( $this->assertFalse(
$request->isHEAD(), $request->isDELETE(),
'POST with valid method override to HEAD' 'DELETE _method override is not honored.'
); );
}
$request = new SS_HTTPRequest( public function detectMethodDataProvider()
'POST', {
'admin/crm', return array(
array('_method' => 'head') '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() { public function testRequestVars() {
$getVars = array( $getVars = array(
'first' => 'a', 'first' => 'a',