diff --git a/admin/code/LeftAndMain.php b/admin/code/LeftAndMain.php index 40198e6c4..5e379f327 100644 --- a/admin/code/LeftAndMain.php +++ b/admin/code/LeftAndMain.php @@ -449,6 +449,9 @@ class LeftAndMain extends Controller implements PermissionProvider { $title = $this->Title(); if(!$response->getHeader('X-Controller')) $response->addHeader('X-Controller', $this->class); if(!$response->getHeader('X-Title')) $response->addHeader('X-Title', urlencode($title)); + + // Prevent clickjacking, see https://developer.mozilla.org/en-US/docs/HTTP/X-Frame-Options + $this->response->addHeader('X-Frame-Options', 'SAMEORIGIN'); return $response; } diff --git a/admin/templates/CMSBreadcrumbs.ss b/admin/templates/CMSBreadcrumbs.ss index 9ef793482..e0d9f13e5 100644 --- a/admin/templates/CMSBreadcrumbs.ss +++ b/admin/templates/CMSBreadcrumbs.ss @@ -1,15 +1,15 @@
-<% loop Buttons %>
- <% if Type = button %>
+<% loop $Buttons %>
+ <% if $Type = button %>
- <% else_if Type = dropdown %>
+ <% else_if $Type = dropdown %>
- <% else_if Type = separator %>
+ <% else_if $Type = separator %>
- <% else_if Type = break %>
+ <% else_if $Type = break %>
<% end_if %> <% end_loop %> diff --git a/admin/templates/Includes/LeftAndMain_EditForm.ss b/admin/templates/Includes/LeftAndMain_EditForm.ss index 1274f741e..3de5007ec 100644 --- a/admin/templates/Includes/LeftAndMain_EditForm.ss +++ b/admin/templates/Includes/LeftAndMain_EditForm.ss @@ -1,19 +1,19 @@ -<% if IncludeFormTag %> +<% if $IncludeFormTag %> <% end_if %> diff --git a/admin/templates/Includes/LeftAndMain_Menu.ss b/admin/templates/Includes/LeftAndMain_Menu.ss index ce11e587e..1f1da0c03 100644 --- a/admin/templates/Includes/LeftAndMain_Menu.ss +++ b/admin/templates/Includes/LeftAndMain_Menu.ss @@ -2,18 +2,18 @@
- $ApplicationName <% if CMSVersion %>$CMSVersion<% end_if %>
+ $ApplicationName <% if $CMSVersion %>$CMSVersion<% end_if %>
- <% if SiteConfig %>$SiteConfig.Title<% else %>$ApplicationName<% end_if %>
+ <% if $SiteConfig %>$SiteConfig.Title<% else %>$ApplicationName<% end_if %>
<% _t('LeftAndMain_Menu.ss.LOGOUT','Log out') %>
- <% with CurrentMember %>
+ <% with $CurrentMember %>
<% _t('LeftAndMain_Menu.ss.Hello','Hi') %>
- <% if FirstName && Surname %>$FirstName $Surname<% else_if FirstName %>$FirstName<% else %>$Email<% end_if %>
+ <% if $FirstName && $Surname %>$FirstName $Surname<% else_if $FirstName %>$FirstName<% else %>$Email<% end_if %>
<% end_with %>
@@ -22,9 +22,9 @@
diff --git a/admin/templates/Includes/ModelAdmin_Content.ss b/admin/templates/Includes/ModelAdmin_Content.ss
index 7f985ad1e..e5d160f6d 100644
--- a/admin/templates/Includes/ModelAdmin_Content.ss
+++ b/admin/templates/Includes/ModelAdmin_Content.ss
@@ -4,7 +4,7 @@
<% include CMSSectionIcon %>
- <% if SectionTitle %>
+ <% if $SectionTitle %>
$SectionTitle
<% else %>
<% _t('ModelAdmin.Title', 'Data Models') %>
@@ -14,7 +14,7 @@
<% sprintf(_t('ModelAdmin_ImportSpec.ss.IMPORTSPECTITLE', 'Specification for %s'),$ModelName) %><% _t('ModelAdmin_ImportSpec.ss.IMPORTSPECFIELDS', 'Database columns') %>- <% loop Fields %> + <% loop $Fields %><% _t('ModelAdmin_ImportSpec.ss.IMPORTSPECRELATIONS', 'Relations') %>- <% loop Relations %> + <% loop $Relations %><% _t('ModelAdmin_Tools.ss.FILTER', 'Filter') %>$SearchForm - <% if ImportForm %> + <% if $ImportForm %><% _t('ModelAdmin_Tools.ss.IMPORT', 'Import') %>$ImportForm <% end_if %> diff --git a/admin/templates/ModelSidebar.ss b/admin/templates/ModelSidebar.ss index 9ba8f15fa..705708140 100644 --- a/admin/templates/ModelSidebar.ss +++ b/admin/templates/ModelSidebar.ss @@ -1,7 +1,7 @@<% _t('ModelSidebar.ss.SEARCHLISTINGS','Search') %>$SearchForm -<% if ImportForm %> +<% if $ImportForm %><% _t('ModelSidebar.ss.IMPORT_TAB_HEADER', 'Import') %>$ImportForm -<% end_if %> \ No newline at end of file +<% end_if %> diff --git a/api/RestfulService.php b/api/RestfulService.php index 05913c3f5..5c95d451d 100644 --- a/api/RestfulService.php +++ b/api/RestfulService.php @@ -352,7 +352,7 @@ class RestfulService extends ViewableData { $match[1] = preg_replace_callback( '/(?<=^|[\x09\x20\x2D])./', create_function('$matches', 'return strtoupper($matches[0]);'), - strtolower(trim($match[1])) + trim($match[1]) ); if( isset($headers[$match[1]]) ) { if (!is_array($headers[$match[1]])) { diff --git a/core/startup/ErrorControlChain.php b/core/startup/ErrorControlChain.php new file mode 100644 index 000000000..9cd344ee0 --- /dev/null +++ b/core/startup/ErrorControlChain.php @@ -0,0 +1,121 @@ +then($callback1)->then($callback2)->then(true, $callback3)->execute(); + * + * WARNING: This class is experimental and designed specifically for use pre-startup in main.php + * It will likely be heavily refactored before the release of 3.2 + */ +class ErrorControlChain { + protected $error = false; + protected $steps = array(); + + protected $suppression = true; + + /** We can't unregister_shutdown_function, so this acts as a flag to enable handling */ + protected $handleFatalErrors = false; + + public function hasErrored() { + return $this->error; + } + + public function setErrored($error) { + $this->error = (bool)$error; + } + + public function setSuppression($suppression) { + $this->suppression = (bool)$suppression; + } + + /** + * Add this callback to the chain of callbacks to call along with the state + * that $error must be in this point in the chain for the callback to be called + * + * @param $callback - The callback to call + * @param $onErrorState - false if only call if no errors yet, true if only call if already errors, null for either + * @return $this + */ + public function then($callback, $onErrorState = false) { + $this->steps[] = array( + 'callback' => $callback, + 'onErrorState' => $onErrorState + ); + return $this; + } + + public function thenWhileGood($callback) { + return $this->then($callback, false); + } + + public function thenIfErrored($callback) { + return $this->then($callback, true); + } + + public function thenAlways($callback) { + return $this->then($callback, null); + } + + public function handleError() { + if ($this->suppression && defined('BASE_URL')) throw new Exception('Generic Error'); + else return false; + } + + protected function lastErrorWasFatal() { + $error = error_get_last(); + return $error && $error['type'] == 1; + } + + public function handleFatalError() { + if ($this->handleFatalErrors && $this->suppression && defined('BASE_URL')) { + if ($this->lastErrorWasFatal()) { + ob_clean(); + $this->error = true; + $this->step(); + } + } + } + + public function execute() { + set_error_handler(array($this, 'handleError'), error_reporting()); + register_shutdown_function(array($this, 'handleFatalError')); + $this->handleFatalErrors = true; + + $this->step(); + } + + protected function step() { + if ($this->steps) { + $step = array_shift($this->steps); + + if ($step['onErrorState'] === null || $step['onErrorState'] === $this->error) { + try { + call_user_func($step['callback'], $this); + } + catch (Exception $e) { + if ($this->suppression && defined('BASE_URL')) $this->error = true; + else throw $e; + } + } + + $this->step(); + } + else { + // Now clean up + $this->handleFatalErrors = false; + restore_error_handler(); + } + } +} diff --git a/core/startup/ParameterConfirmationToken.php b/core/startup/ParameterConfirmationToken.php new file mode 100644 index 000000000..e0ccf9d87 --- /dev/null +++ b/core/startup/ParameterConfirmationToken.php @@ -0,0 +1,110 @@ +randomToken('md5'); + + // Store a file in the session save path (safer than /tmp, as open_basedir might limit that) + file_put_contents($this->pathForToken($token), $token); + + return $token; + } + + protected function checkToken($token) { + $file = $this->pathForToken($token); + $content = null; + + if (file_exists($file)) { + $content = file_get_contents($file); + unlink($file); + } + + return $content == $token; + } + + public function __construct($parameterName) { + // Store the parameter name + $this->parameterName = $parameterName; + // Store the parameter value + $this->parameter = isset($_GET[$parameterName]) ? $_GET[$parameterName] : null; + // Store the token + $this->token = isset($_GET[$parameterName.'token']) ? $_GET[$parameterName.'token'] : null; + + // If a token was provided, but isn't valid, ignore it + if ($this->token && (!$this->checkToken($this->token))) $this->token = null; + } + + public function parameterProvided() { + return $this->parameter !== null; + } + + public function tokenProvided() { + return $this->token !== null; + } + + public function params() { + return array( + $this->parameterName => $this->parameter, + $this->parameterName.'token' => $this->genToken() + ); + } + + public function reloadWithToken() { + global $url; + + // Are we http or https? + $proto = 'http'; + + if(isset($_SERVER['HTTP_X_FORWARDED_PROTOCOL'])) { + if(strtolower($_SERVER['HTTP_X_FORWARDED_PROTOCOL']) == 'https') $proto = 'https'; + } + + if((!empty($_SERVER['HTTPS']) && $_SERVER['HTTPS'] != 'off')) $proto = 'https'; + if(isset($_SERVER['SSL'])) $proto = 'https'; + + // What's our host + $host = $_SERVER['HTTP_HOST']; + + // What's our GET params (ensuring they include the original parameter + a new token) + $params = array_merge($_GET, $this->params()); + unset($params['url']); + + // Join them all together into the original URL + $location = "$proto://" . $host . BASE_URL . $url . ($params ? '?'.http_build_query($params) : ''); + + // And redirect + header('location: '.$location, true, 302); + die; + } +} diff --git a/dev/BehatFixtureFactory.php b/dev/BehatFixtureFactory.php index b4ce190de..a4f7a829a 100644 --- a/dev/BehatFixtureFactory.php +++ b/dev/BehatFixtureFactory.php @@ -9,7 +9,7 @@ class BehatFixtureFactory extends \FixtureFactory { // Copy identifier to some visible property unless its already defined. // Exclude files, since they generate their own named based on the file path. - if(!is_a($name, 'File', true)) { + if(!$name != 'File' && !is_subclass_of($name, 'File')) { foreach(array('Name', 'Title') as $fieldName) { if(singleton($name)->hasField($fieldName) && !isset($data[$fieldName])) { $data[$fieldName] = $identifier; diff --git a/docs/en/changelogs/3.0.6.md b/docs/en/changelogs/3.0.6.md index 7e6448cf0..ba7e28704 100644 --- a/docs/en/changelogs/3.0.6.md +++ b/docs/en/changelogs/3.0.6.md @@ -1,5 +1,32 @@ # 3.0.6 (Not yet released) +## Overview + + * Security: Require ADMIN for `?flush=1` (stop denial of service attacks) + ([#1692](https://github.com/silverstripe/silverstripe-framework/issues/1692)) + +## Details + +### Security: Require ADMIN for ?flush=1 + +Flushing the various manifests (class, template, config) is performed through a GET +parameter (`flush=1`). Since this action requires more server resources than normal requests, +it can facilitate [denial-of-service attacks](https://en.wikipedia.org/wiki/Denial-of-service_attack). + +To prevent this, main.php now checks and only allows the flush parameter in the following cases: + + * The [environment](/topics/environment-management) is in "dev mode" + * A user is logged in with ADMIN permissions + * An error occurs during startup + +This applies to both `flush=1` and `flush=all` (technically we only check for the existence of any parameter value) +but only through web requests made through main.php - CLI requests, or any other request that goes through +a custom start up script will still process all flush requests as normal. + ## Upgrading - * If you have created your own composite database fields, then you shoulcd amend the setValue() to allow the passing of an object (usually DataObject) as well as an array. + * If you have created your own composite database fields, then you should amend the setValue() to allow the passing of + an object (usually DataObject) as well as an array. + + * If you have provided your own startup scripts (ones that include core/Core.php) that can be accessed via a web + request, you should ensure that you limit use of the flush parameter diff --git a/docs/en/changelogs/3.1.0.md b/docs/en/changelogs/3.1.0.md index 8795f02e5..996ab1c6c 100644 --- a/docs/en/changelogs/3.1.0.md +++ b/docs/en/changelogs/3.1.0.md @@ -18,6 +18,8 @@ ### Framework + * Security: Require ADMIN for `?flush=1` (stop denial of service attacks) + ([#1692](https://github.com/silverstripe/silverstripe-framework/issues/1692)) * Static properties are immutable and private, you must use Config API * Statics in custom Page classes need to be "private" * `$default_cast` is now `Text` instead of `HTMLText`, to secure templates from XSS by default @@ -37,6 +39,24 @@ * Support for [Composer](http://getcomposer.org) dependency manager (also works with 3.0) * Added support for filtering incoming HTML from TinyMCE (disabled by default, see [security](/topics/security)) +## Details + +### Security: Require ADMIN for ?flush=1 + +Flushing the various manifests (class, template, config) is performed through a GET +parameter (`flush=1`). Since this action requires more server resources than normal requests, +it can facilitate [denial-of-service attacks](https://en.wikipedia.org/wiki/Denial-of-service_attack). + +To prevent this, main.php now checks and only allows the flush parameter in the following cases: + + * The [environment](/topics/environment-management) is in "dev mode" + * A user is logged in with ADMIN permissions + * An error occurs during startup + +This applies to both `flush=1` and `flush=all` (technically we only check for the existence of any parameter value) +but only through web requests made through main.php - CLI requests, or any other request that goes through +a custom start up script will still process all flush requests as normal. + ## Upgrading ### Statics in custom Page classes need to be "private" diff --git a/docs/en/reference/grid-field.md b/docs/en/reference/grid-field.md index 06326568b..d21f443a4 100644 --- a/docs/en/reference/grid-field.md +++ b/docs/en/reference/grid-field.md @@ -154,7 +154,7 @@ The fields displayed in the edit form are from `DataObject::getCMSFields()` The `GridFieldDetailForm` component drives the record editing form which is usually configured through the configs `GridFieldConfig_RecordEditor` and `GridFieldConfig_RelationEditor` described above. It takes its fields from `DataObject->getCMSFields()`, -but can be customized to accept different fields via its `[api:GridFieldDetailForm->setFields()](api:setFields())` method. +but can be customized to accept different fields via its `[api:GridFieldDetailForm->setFields()]` method. The component also has the ability to load and save data stored on join tables when two records are related via a "many_many" relationship, as defined through diff --git a/docs/en/reference/urlvariabletools.md b/docs/en/reference/urlvariabletools.md index c4fe6b5c8..30cbe65b4 100644 --- a/docs/en/reference/urlvariabletools.md +++ b/docs/en/reference/urlvariabletools.md @@ -17,10 +17,8 @@ Append the option and corresponding value to your URL in your browser's address | URL Variable | | Values | | Description | | ------------ | | ------ | | ----------- | - | flush | | 1,all | | This will clear out all cached information about the page. This is used frequently during development - for example, when adding new PHP or SS files. See below for value descriptions. | + | flush=1 | | 1 | | Clears out all caches. Used mainly during development, e.g. when adding new classes or templates. Requires "dev" mode or ADMIN login | | showtemplate | | 1 | | Show the compiled version of all the templates used, including line numbers. Good when you have a syntax error in a template. Cannot be used on a Live site without **isDev**. **flush** can be used with the following values: | - | ?flush=1 | | | | Flushes the current page and included templates | - | ?flush=all | | | | Flushes the entire template cache | ## General Testing diff --git a/docs/en/topics/caching.md b/docs/en/topics/caching.md new file mode 100644 index 000000000..61a6a4225 --- /dev/null +++ b/docs/en/topics/caching.md @@ -0,0 +1,26 @@ +# Caching + +## Built-In Caches + +The framework uses caches to store infrequently changing values. +By default, the storage mechanism is simply the filesystem, although +other cache backends can be configured. All caches use the `[api:SS_Cache]` API. + +The most common caches are manifests of various resources: + + * PHP class locations (`[api:SS_ClassManifest]`) + * Template file locations and compiled templates (`[api:SS_TemplateManifest]`) + * Configuration settings from YAML files (`[api:SS_ConfigManifest]`) + * Language files (`[api:i18n]`) + +Flushing the various manifests is performed through a GET +parameter (`flush=1`). Since this action requires more server resources than normal requests, +executing the action is limited to the following cases when performed via a web request: + + * The [environment](/topics/environment-management) is in "dev mode" + * A user is logged in with ADMIN permissions + * An error occurs during startup + +## Custom Caches + +See `[api:SS_Cache]`. \ No newline at end of file diff --git a/docs/en/topics/index.md b/docs/en/topics/index.md index d192ed1d3..e102bb2b4 100644 --- a/docs/en/topics/index.md +++ b/docs/en/topics/index.md @@ -4,6 +4,7 @@ This section provides an overview on how things fit together, the "conceptual gl It is where most documentation should live, and is the natural "second step" after finishing the tutorials. * [Access Control and Page Security](access-control): Restricting access and setting up permissions on your website + * [Caching](caching): Explains built-in caches for classes, config and templates. How to use your own caches. * [Command line Usage](commandline): Calling controllers via the command line interface using `sake` * [Configuration](configuration): Influence behaviour through PHP and YAML configuration * [Controller](controller): The intermediate layer between your templates and the data model diff --git a/docs/en/topics/security.md b/docs/en/topics/security.md index acc9ba89a..812243f25 100644 --- a/docs/en/topics/security.md +++ b/docs/en/topics/security.md @@ -127,6 +127,38 @@ or [sanitize](http://htmlpurifier.org/) it correctly. See [http://shiflett.org/articles/foiling-cross-site-attacks](http://shiflett.org/articles/foiling-cross-site-attacks) for in-depth information about "Cross-Site-Scripting". +### What if I can't trust my editors? + +The default configuration of SilverStripe assumes some level of trust is given to your editors who have access +to the CMS. Though the HTML WYSIWYG editor is configured to provide some control over the HTML an editor provides, +this is not enforced server side, and so can be bypassed by a malicious editor. A editor that does so can use an +XSS attack against an admin to perform any administrative action. + +If you can't trust your editors, SilverStripe must be configured to filter the content so that any javascript is +stripped out + +To enable filtering, set the HtmlEditorField::$sanitise_server_side [configuration](/topics/configuration) property to +true, e.g. + + HtmlEditorField::config()->sanitise_server_side = true + +The built in sanitiser enforces the TinyMCE whitelist rules on the server side, and is sufficient to eliminate the +most common XSS vectors. + +However some subtle XSS attacks that exploit HTML parsing bugs need heavier filtering. For greater protection +you can install the [htmlpurifier](https://github.com/silverstripe-labs/silverstripe-htmlpurifier) module which +will replace the built in sanitiser with one that uses the [HTML Purifier](http://htmlpurifier.org/) library. + +In both cases, you must ensure that you have not configured TinyMCE to explicitly allow script elements or other +javascript-specific attributes. + +##### But I also need my editors to provide javascript + +It is not currently possible to allow editors to provide javascript content and yet still protect other users +from any malicious code within that javascript. + +We recommend configuring [shortcodes](/reference/shortcodes) that can be used by editors in place of using javascript directly. + ### Escaping model properties `[api:SSViewer]` (the SilverStripe template engine) automatically takes care of escaping HTML tags from specific @@ -377,11 +409,62 @@ you need to serve directly. See [Apache](/installation/webserver) and [Nginx](/installation/nginx) installation documentation for details specific to your web server +See [Apache](/installation/webserver) and [Nginx](/installation/nginx) installation documentation for details specific to your web server + +## Passwords + +SilverStripe stores passwords with a strong hashing algorithm (blowfish) by default +(see [api:PasswordEncryptor]). It adds randomness to these hashes via +salt values generated with the strongest entropy generators available on the platform +(see [api:RandomGenerator]). This prevents brute force attacks with +[Rainbow tables](http://en.wikipedia.org/wiki/Rainbow_table). + +Strong passwords are a crucial part of any system security. +So in addition to storing the password in a secure fashion, +you can also enforce specific password policies by configuring +a [api:PasswordValidator]: + + :::php + $validator = new PasswordValidator(); + $validator->minLength(7); + $validator->checkHistoricalPasswords(6); + $validator->characterStrength('lowercase','uppercase','digits','punctuation'); + Member::set_password_validator($validator); + +In addition, you can tighten password security with the following configuration settings: + + * `Member.password_expiry_days`: Set the number of days that a password should be valid for. + * `Member.lock_out_after_incorrect_logins`: Number of incorrect logins after which + the user is blocked from further attempts for the timespan defined in `$lock_out_delay_mins` + * `Member.lock_out_delay_mins`: Minutes of enforced lockout after incorrect password attempts. + Only applies if `lock_out_after_incorrect_logins` is greater than 0. + +## Clickjacking: Prevent iframe Inclusion + +"[Clickjacking](http://en.wikipedia.org/wiki/Clickjacking)" is a malicious technique +where a web user is tricked into clicking on hidden interface elements, which can +lead to the attacker gaining access to user data or taking control of the website behaviour. + +You can signal to browsers that the current response isn't allowed to be +included in HTML "frame" or "iframe" elements, and thereby prevent the most common +attack vector. This is done through a HTTP header, which is usually added in your +controller's `init()` method: + + :::php + class MyController extends Controller { + public function init() { + parent::init(); + + $this->response->addHeader('X-Frame-Options', 'SAMEORIGIN'); + } + } + + +This is a recommended option to secure any controller which displays +or submits sensitive user input, and is enabled by default in all CMS controllers, +as well as the login form. ## Related * [http://silverstripe.org/security-releases/](http://silverstripe.org/security-releases/) - -## Links - * [Best-practices for securing MySQL (securityfocus.com)](http://www.securityfocus.com/infocus/1726) diff --git a/forms/UploadField.php b/forms/UploadField.php index a3d265682..90e004676 100644 --- a/forms/UploadField.php +++ b/forms/UploadField.php @@ -1195,6 +1195,7 @@ class UploadField extends FileField { * * @param SS_HTTPRequest $request * @return SS_HTTPResponse + * @return SS_HTTPResponse */ public function upload(SS_HTTPRequest $request) { if($this->isDisabled() || $this->isReadonly() || !$this->canUpload()) { @@ -1222,6 +1223,7 @@ class UploadField extends FileField { // Format response with json $response = new SS_HTTPResponse(Convert::raw2json(array($return))); $response->addHeader('Content-Type', 'text/plain'); + if(!empty($return['error'])) $response->setStatusCode(403); return $response; } diff --git a/lang/en.yml b/lang/en.yml index 64df41040..154dc2ad4 100644 --- a/lang/en.yml +++ b/lang/en.yml @@ -367,6 +367,7 @@ en: EMPTYNEWPASSWORD: 'The new password can''t be empty, please try again' ENTEREMAIL: 'Please enter an email address to get a password reset link.' ERRORLOCKEDOUT: 'Your account has been temporarily disabled because of too many failed attempts at logging in. Please try again in 20 minutes.' + ERRORLOCKEDOUT2: 'Your account has been temporarily disabled because of too many failed attempts at logging in. Please try again in {count} minutes.' ERRORNEWPASSWORD: 'You have entered your new password differently, try again' ERRORPASSWORDNOTMATCH: 'Your current password does not match, please try again' ERRORWRONGCRED: 'That doesn''t seem to be the right e-mail address or password. Please try again.' diff --git a/main.php b/main.php index 1f98f7e82..90512b68e 100644 --- a/main.php +++ b/main.php @@ -56,52 +56,113 @@ if (version_compare(phpversion(), '5.3.2', '<')) { /** * Include SilverStripe's core code */ -require_once('core/Core.php'); +require_once('core/startup/ErrorControlChain.php'); +require_once('core/startup/ParameterConfirmationToken.php'); -// IIS will sometimes generate this. -if(!empty($_SERVER['HTTP_X_ORIGINAL_URL'])) { - $_SERVER['REQUEST_URI'] = $_SERVER['HTTP_X_ORIGINAL_URL']; -} +$chain = new ErrorControlChain(); +$token = new ParameterConfirmationToken('flush'); -// PHP 5.4's built-in webserver uses this -if (php_sapi_name() == 'cli-server') { - $url = $_SERVER['REQUEST_URI']; - - // Querystring args need to be explicitly parsed - if(strpos($url,'?') !== false) { - list($url, $query) = explode('?',$url,2); - parse_str($query, $_GET); - if ($_GET) $_REQUEST = array_merge((array)$_REQUEST, (array)$_GET); - } - - // Pass back to the webserver for files that exist - if(file_exists(BASE_PATH . $url)) return false; +$chain + // First, if $_GET['flush'] was set, but no valid token, suppress the flush + ->then(function($chain) use ($token){ + if (isset($_GET['flush']) && !$token->tokenProvided()) { + unset($_GET['flush']); + } + else { + $chain->setSuppression(false); + } + }) + // Then load in core + ->then(function(){ + require_once('core/Core.php'); + }) + // Then build the URL (even if Core didn't load beyond setting BASE_URL) + ->thenAlways(function(){ + global $url; -// Apache rewrite rules use this -} else if (isset($_GET['url'])) { - $url = $_GET['url']; - // IIS includes get variables in url - $i = strpos($url, '?'); - if($i !== false) { - $url = substr($url, 0, $i); - } - -// Lighttpd uses this -} else { - if(strpos($_SERVER['REQUEST_URI'],'?') !== false) { - list($url, $query) = explode('?', $_SERVER['REQUEST_URI'], 2); - parse_str($query, $_GET); - if ($_GET) $_REQUEST = array_merge((array)$_REQUEST, (array)$_GET); - } else { - $url = $_SERVER["REQUEST_URI"]; - } -} + // IIS will sometimes generate this. + if(!empty($_SERVER['HTTP_X_ORIGINAL_URL'])) { + $_SERVER['REQUEST_URI'] = $_SERVER['HTTP_X_ORIGINAL_URL']; + } -// Remove base folders from the URL if webroot is hosted in a subfolder -if (substr(strtolower($url), 0, strlen(BASE_URL)) == strtolower(BASE_URL)) $url = substr($url, strlen(BASE_URL)); + // PHP 5.4's built-in webserver uses this + if (php_sapi_name() == 'cli-server') { + $url = $_SERVER['REQUEST_URI']; -// Connect to database -require_once('model/DB.php'); + // Querystring args need to be explicitly parsed + if(strpos($url,'?') !== false) { + list($url, $query) = explode('?',$url,2); + parse_str($query, $_GET); + if ($_GET) $_REQUEST = array_merge((array)$_REQUEST, (array)$_GET); + } + + // Apache rewrite rules use this + } else if (isset($_GET['url'])) { + $url = $_GET['url']; + // IIS includes get variables in url + $i = strpos($url, '?'); + if($i !== false) { + $url = substr($url, 0, $i); + } + + // Lighttpd uses this + } else { + if(strpos($_SERVER['REQUEST_URI'],'?') !== false) { + list($url, $query) = explode('?', $_SERVER['REQUEST_URI'], 2); + parse_str($query, $_GET); + if ($_GET) $_REQUEST = array_merge((array)$_REQUEST, (array)$_GET); + } else { + $url = $_SERVER["REQUEST_URI"]; + } + } + + // Remove base folders from the URL if webroot is hosted in a subfolder + if (substr(strtolower($url), 0, strlen(BASE_URL)) == strtolower(BASE_URL)) $url = substr($url, strlen(BASE_URL)); + }) + // Then start up the database + ->then(function(){ + require_once('model/DB.php'); + global $databaseConfig; + if ($databaseConfig) DB::connect($databaseConfig); + }) + // Then if a flush was requested, redirect to it + ->then(function($chain) use ($token){ + if ($token->parameterProvided() && !$token->tokenProvided()) { + // First, check if we're in dev mode, or the database doesn't have any security data + $canFlush = Director::isDev() || !Security::database_is_ready(); + + // Otherwise, we start up the session if needed, then check for admin + if (!$canFlush) { + if(!isset($_SESSION) && (isset($_COOKIE[session_name()]) || isset($_REQUEST[session_name()]))) { + Session::start(); + } + + if (Permission::check('ADMIN')) { + $canFlush = true; + } + else { + $loginPage = Director::absoluteURL(Config::inst()->get('Security', 'login_url')); + $loginPage .= "?BackURL=" . urlencode($_SERVER['REQUEST_URI']); + + header('location: '.$loginPage, true, 302); + die; + } + } + + // And if we can flush, reload with an authority token + if ($canFlush) $token->reloadWithToken(); + } + }) + // Finally if a flush was requested but there was an error while figuring out if it's allowed, do it anyway + ->thenIfErrored(function() use ($token){ + if ($token->parameterProvided() && !$token->tokenProvided()) { + $token->reloadWithToken(); + } + }) + ->execute(); + +// If we're in PHP's built in webserver, pass back to the webserver for files that exist +if (php_sapi_name() == 'cli-server' && file_exists(BASE_PATH . $url) && is_file(BASE_PATH . $url)) return false; global $databaseConfig; @@ -112,17 +173,15 @@ if(!isset($databaseConfig) || !isset($databaseConfig['database']) || !$databaseC } $s = (isset($_SERVER['SSL']) || (isset($_SERVER['HTTPS']) && $_SERVER['HTTPS'] != 'off')) ? 's' : ''; $installURL = "http$s://" . $_SERVER['HTTP_HOST'] . BASE_URL . '/install.php'; - + // The above dirname() will equate to "\" on Windows when installing directly from http://localhost (not using // a sub-directory), this really messes things up in some browsers. Let's get rid of the backslashes $installURL = str_replace('\\', '', $installURL); - + header("Location: $installURL"); die(); } -DB::connect($databaseConfig); - // Direct away - this is the "main" function, that hands control to the appropriate controller DataModel::set_inst(new DataModel()); Director::direct($url, DataModel::inst()); diff --git a/model/Aggregate.php b/model/Aggregate.php index f0dce851b..372843299 100644 --- a/model/Aggregate.php +++ b/model/Aggregate.php @@ -29,6 +29,8 @@ * NOTE: The cache logic uses tags, and so a backend that supports tags is required. Currently only the File * backend (and the two-level backend with the File backend as the slow store) meets this requirement * + * @deprecated 3.1 Use DataList to aggregate data + * * @author hfried * @package framework * @subpackage core @@ -60,10 +62,15 @@ class Aggregate extends ViewableData { /** * Constructor * + * @deprecated 3.1 Use DataList to aggregate data + * * @param string $type The DataObject type we are building an aggregate for * @param string $filter (optional) An SQL filter to apply to the selected rows before calculating the aggregate */ public function __construct($type, $filter = '') { + Deprecation::notice('3.1', 'Call aggregate methods on a DataList directly instead. In templates' + . ' an example of the new syntax is <% cached List(Member).max(LastEdited) %> instead' + . ' (check partial-caching.md documentation for more details.)'); $this->type = $type; $this->filter = $filter; parent::__construct(); diff --git a/model/DataObject.php b/model/DataObject.php index 1d6fbb331..fcada2eca 100644 --- a/model/DataObject.php +++ b/model/DataObject.php @@ -1468,39 +1468,11 @@ class DataObject extends ViewableData implements DataObjectInterface, i18nEntity } /** - * Get the query object for a $has_many Component. - * - * @param string $componentName - * @param string $filter - * @param string|array $sort - * @param string $join Deprecated, use leftJoin($table, $joinClause) instead - * @param string|array $limit - * @return SQLQuery + * @deprecated 3.1 Use getComponents to get a filtered DataList for an object's relation */ public function getComponentsQuery($componentName, $filter = "", $sort = "", $join = "", $limit = "") { - if(!$componentClass = $this->has_many($componentName)) { - user_error("DataObject::getComponentsQuery(): Unknown 1-to-many component '$componentName'" - . " on class '$this->class'", E_USER_ERROR); - } - - if($join) { - throw new \InvalidArgumentException( - 'The $join argument has been removed. Use leftJoin($table, $joinClause) instead.' - ); - } - - $joinField = $this->getRemoteJoinField($componentName, 'has_many'); - - $id = $this->getField("ID"); - - // get filter - $combinedFilter = "\"$joinField\" = '$id'"; - if(!empty($filter)) $combinedFilter .= " AND ({$filter})"; - - return DataList::create($componentClass) - ->where($combinedFilter) - ->canSortBy($sort) - ->limit($limit); + Deprecation::notice('3.1', "Use getComponents to get a filtered DataList for an object's relation"); + return $this->getComponents($componentName, $filter, $sort, $join, $limit); } /** diff --git a/model/fieldtypes/ForeignKey.php b/model/fieldtypes/ForeignKey.php index 3fe7b07f3..1c51529f3 100644 --- a/model/fieldtypes/ForeignKey.php +++ b/model/fieldtypes/ForeignKey.php @@ -7,7 +7,7 @@ * @uses DropdownField * * @param string $name - * @param DataOject $object The object that the foreign key is stored on (should have a relation with $name) + * @param DataObject $object The object that the foreign key is stored on (should have a relation with $name) * * @package framework * @subpackage model diff --git a/security/Member.php b/security/Member.php index f5165693e..7c88d4616 100644 --- a/security/Member.php +++ b/security/Member.php @@ -113,9 +113,18 @@ class Member extends DataObject implements TemplateGlobalProvider { /** * @config - * @var Int + * @var Int Number of incorrect logins after which + * the user is blocked from further attempts for the timespan + * defined in {@link $lock_out_delay_mins}. */ private static $lock_out_after_incorrect_logins = null; + + /** + * @config + * @var integer Minutes of enforced lockout after incorrect password attempts. + * Only applies if {@link $lock_out_after_incorrect_logins} greater than 0. + */ + private static $lock_out_delay_mins = 15; /** * @config @@ -210,6 +219,9 @@ class Member extends DataObject implements TemplateGlobalProvider { public function checkPassword($password) { $result = $this->canLogIn(); + // Short-circuit the result upon failure, no further checks needed. + if (!$result->valid()) return $result; + if(empty($this->Password) && $this->exists()) { $result->error(_t('Member.NoPassword','There is no password on this member.')); return $result; @@ -238,11 +250,15 @@ class Member extends DataObject implements TemplateGlobalProvider { $result = ValidationResult::create(); if($this->isLockedOut()) { - $result->error(_t ( - 'Member.ERRORLOCKEDOUT', - 'Your account has been temporarily disabled because of too many failed attempts at ' . - 'logging in. Please try again in 20 minutes.' - )); + $result->error( + _t( + 'Member.ERRORLOCKEDOUT2', + 'Your account has been temporarily disabled because of too many failed attempts at ' . + 'logging in. Please try again in {count} minutes.', + null, + array('count' => $this->config()->lock_out_delay_mins) + ) + ); } $this->extend('canLogIn', $result); @@ -429,7 +445,9 @@ class Member extends DataObject implements TemplateGlobalProvider { self::session_regenerate_id(); Session::set("loggedInAs", $member->ID); // This lets apache rules detect whether the user has logged in - if(Member::config()->login_marker_cookie) Cookie::set(Member::config()->login_marker_cookie, 1, 0, null, null, false, true); + if(Member::config()->login_marker_cookie) { + Cookie::set(Member::config()->login_marker_cookie, 1, 0, null, null, false, true); + } $generator = new RandomGenerator(); $token = $generator->randomToken('sha1'); @@ -717,7 +735,8 @@ class Member extends DataObject implements TemplateGlobalProvider { $encryption_details = Security::encrypt_password( $this->Password, // this is assumed to be cleartext $this->Salt, - ($this->PasswordEncryption) ? $this->PasswordEncryption : Security::config()->password_encryption_algorithm, + ($this->PasswordEncryption) ? + $this->PasswordEncryption : Security::config()->password_encryption_algorithm, $this ); @@ -1407,7 +1426,8 @@ class Member extends DataObject implements TemplateGlobalProvider { $this->write(); if($this->FailedLoginCount >= self::config()->lock_out_after_incorrect_logins) { - $this->LockedOutUntil = date('Y-m-d H:i:s', time() + 15*60); + $lockoutMins = self::config()->lock_out_delay_mins; + $this->LockedOutUntil = date('Y-m-d H:i:s', time() + $lockoutMins*60); $this->write(); } } diff --git a/security/Security.php b/security/Security.php index 0b3741ef3..155912363 100644 --- a/security/Security.php +++ b/security/Security.php @@ -270,6 +270,13 @@ class Security extends Controller { return; } + public function init() { + parent::init(); + + // Prevent clickjacking, see https://developer.mozilla.org/en-US/docs/HTTP/X-Frame-Options + $this->response->addHeader('X-Frame-Options', 'SAMEORIGIN'); + } + /** * Get the login form to process according to the submitted data diff --git a/templates/Image_iframe.ss b/templates/Image_iframe.ss index 10cbc33ab..a96117159 100644 --- a/templates/Image_iframe.ss +++ b/templates/Image_iframe.ss @@ -8,17 +8,17 @@
- <% if UseSimpleForm %>
+ <% if $UseSimpleForm %>
$EditImageSimpleForm
<% else %>
$EditImageForm
<% end_if %>
- <% if Image.ID %>
+ <% if $Image.ID %>
$Image.CMSThumbnail
- <% if DeleteImageForm %>
+ <% if $DeleteImageForm %>
$DeleteImageForm
<% end_if %>
@@ -26,4 +26,4 @@
- |