mirror of
https://github.com/silverstripe/silverstripe-framework
synced 2024-10-22 14:05:37 +02:00
350 lines
21 KiB
Markdown
350 lines
21 KiB
Markdown
# 2.4.3 (2010-11-11)
|
|
|
|
## Overview
|
|
|
|
* Fixed a security issue where destructive controller actions are not correctly secured against Cross-Site Request Forgery (CSRF). This affects various CMS interfaces, as well as classes based on TableListField or ComplexTableField.
|
|
* Enhance the protection of the assets/ directory in both IIS and Apache by including a file type whitelist.
|
|
* Compatibility with PHPUnit 3.5
|
|
* Allow direct test execution through the "phpunit" binary, in addition to the existing "sake" executable and TestRunner class.
|
|
* Misc. fixes to validation in date-based form fields, as well as better formatting of dates in non-default locales.
|
|
|
|
## Upgrading Notes
|
|
|
|
### Important: If you are running PHP as FastCGI with Apache
|
|
|
|
Your development environment, or production web host may be running PHP as FastCGI with Apache. If this is the case,
|
|
there is a regression in 2.4.3 which will break your site. There are two ways to resolve this problem:
|
|
|
|
- Don't upgrade to 2.4.3 for now, until 2.4.4 is released
|
|
- Patch the assets/.htaccess file like this: http://open.silverstripe.org/changeset/113809
|
|
|
|
This does **NOT** affect IIS, or other web servers that don't understand .htaccess files.
|
|
|
|
If you're not sure whether PHP is running as FastCGI in your server environment, please check the output of phpinfo().
|
|
The easiest way to do this is to open mysite/_config.php and add phpinfo() to the bottom of the file, then browse to
|
|
your site to see the environment information.
|
|
|
|
Forum references of where the community have had issues:
|
|
|
|
* http://silverstripe.org/installing-silverstripe/show/14878
|
|
* http://www.silverstripe.org/general-questions/show/14861
|
|
|
|
### Important: Add manual request forgery protection to destructive controller actions
|
|
|
|
Cross Site Request Forgery (CSRF) allows an attacker to initiate unauthorized actions against a victim with a valid
|
|
login to a target site in the same browser session, e.g. a login to SilverStripe CMS ([read more about
|
|
CSRF](http://shiflett.org/articles/cross-site-request-forgeries)).
|
|
|
|
While SilverStripe protects form submissions from CSRF automatically, destructive GET and POST actions in subclasses of
|
|
*Controller* are vulnerable without manual checks.
|
|
|
|
**You will need to review any custom subclasses of *RequestHandler* and *Controller* (including subclasses of
|
|
*FormField*), and add manual checks.** Best practice is to return a "400 Bad Request" HTTP error when the CSRF check
|
|
fails.
|
|
|
|
Also review the method signatures of any form actions - these are only protected automatically with the correct count of
|
|
parameters (//$data, $form// instead of *$request*).
|
|
|
|
|
|
:::php
|
|
// Form field actions
|
|
class MyFormField extends FormField {
|
|
|
|
// Form fields always have a reference to their form.
|
|
// Use the form-specific token instance.
|
|
function delete($request) {
|
|
$token = $this->getForm()->getSecurityToken();
|
|
if(!$token->checkRequest($request)) return $this->httpError(400);
|
|
|
|
// valid form field delete action
|
|
}
|
|
}
|
|
|
|
// Controller actions (GET and POST) without form
|
|
class MyController extends Controller {
|
|
|
|
// Manually adds token to link
|
|
function DeleteLink() {
|
|
$token = SecurityToken::inst();
|
|
$link = Controller::join_links($this->Link(), 'delete');
|
|
// will add "?SecurityID=`<random value>`"
|
|
$link = $token->addToUrl($link);
|
|
return $link;
|
|
}
|
|
|
|
// Controller actions pass through the request object,
|
|
// not called through a form.
|
|
// Use a global token instance.
|
|
function delete(SS_HTTPRequest $request) {
|
|
$token = SecurityToken::inst();
|
|
if(!$token->checkRequest($request)) return $this->httpError(400);
|
|
|
|
// valid controller delete action
|
|
}
|
|
}
|
|
|
|
// Controller actions (GET and POST) with form
|
|
class MyController extends Controller {
|
|
|
|
// Forms have CSRF protection turned on by default,
|
|
// will add a HiddenField instance called "SecurityID"
|
|
function Form() {
|
|
return new Form(
|
|
$this, 'Form', new FieldSet(), new FieldSet(new FormAction('submit'))
|
|
);
|
|
}
|
|
|
|
// Form->httpSubmission() checks for CSRF automatically,
|
|
// but you need to include both parameters in the method signature.
|
|
function submit($data, Form $form) {
|
|
// valid submit action
|
|
}
|
|
|
|
}
|
|
|
|
|
|
Note: It is regarded as good practice in HTTP to only have destructive actions in POST submissions rather than GET
|
|
links.
|
|
|
|
If you have overwritten any CMS templates (based on LeftAndMain), you will need to update them to include "SecurityID"
|
|
parameter in all manually created forms.
|
|
|
|
Affected classes and methods:
|
|
|
|
* AssetAdmin->addfolder()
|
|
* AssetAdmin->deletefolder()
|
|
* AssetAdmin->deleteunusedthumbnails()
|
|
* AssetAdmin->removefile()
|
|
* AssetTableField
|
|
* CMSMain->addpage()
|
|
* CMSMain->buildbrokenlinks()
|
|
* CMSMain->createtranslation()
|
|
* CMSMain->duplicate()
|
|
* CMSMain->duplicatewithchildren()
|
|
* CMSMain->publishall()
|
|
* CMSMain->restore()
|
|
* CMSMain->rollback()
|
|
* CMSMain->unpublish()
|
|
* CommentTableField
|
|
* ComplexTableField
|
|
* LeftAndMain->ajaxupdateparent()
|
|
* LeftAndMain->ajaxupdatesort()
|
|
* LeftAndMain->deleteitems()
|
|
* MemberTableField
|
|
* MemberTableField->addtogroup()
|
|
* MemberTableField->delete()
|
|
* MemberTableField_ItemRequest->delete()
|
|
* PageComment
|
|
* PageComment_Controller
|
|
* SecurityAdmin->addgroup()
|
|
* SecurityAdmin->addmember()
|
|
* SecurityAdmin->MemberForm()
|
|
* SecurityAdmin->removememberfromgroup()
|
|
* SecurityAdmin->savemember()
|
|
* TableListField
|
|
|
|
### Usage of Controller::join_links() to concatenate links now mandatory
|
|
|
|
The `[api:Controller::join_links()]` method
|
|
to create links within SilverStripe controllers is now mandatory. This method ensures that links with existing GET
|
|
parameters don't break through string concatenation.
|
|
|
|
:::php
|
|
// bad
|
|
$link = $this->Link() . 'export?csv=1';
|
|
|
|
// good
|
|
$link = Controller::join_links($this->Link(), 'export', '?csv=1');
|
|
|
|
|
|
Full controller example:
|
|
|
|
:::php
|
|
class MyController extends Controller {
|
|
|
|
function export($request) {
|
|
// ...
|
|
}
|
|
|
|
function Link($action = null) {
|
|
return Controller::join_links('MyController', $action);
|
|
}
|
|
|
|
function ExportLink() {
|
|
return Controller::join_links($this->Link('export'), '?csv=1');
|
|
}
|
|
}
|
|
|
|
|
|
Using this method is particularly important for any custom
|
|
`[api:TableListField]` or
|
|
`[api:ComplexTableField]` subclasses and any
|
|
`[api:LeftAndMain]` subclass for the CMS UI. These classes in
|
|
particular were refactored to secure destructive links against Cross Site Request Forgery (CSRF). This is achieved via a
|
|
mandatory "SecurityID" GET parameter appended to the base link.
|
|
|
|
### Auto-disabled "SecurityID" field in FunctionalTest
|
|
|
|
"SecurityID" tokens are now disabled by default in unit tests, to make form submissions easier.
|
|
You can manually enable security tokens, either globally or for a specific form.
|
|
|
|
:::php
|
|
class MyTest extends SapphireTest {
|
|
|
|
// option 1: enable for all forms created through this test
|
|
function setUp() {
|
|
parent::setUp();
|
|
SecurityToken::enable();
|
|
}
|
|
|
|
// option 2: enable for one specific form
|
|
function testMyForm() {
|
|
$form = new MyForm();
|
|
$form->enableSecurityToken();
|
|
}
|
|
}
|
|
|
|
|
|
## 2.4.3 Changelog
|
|
|
|
### Features and Enhancements
|
|
|
|
* [rev:113420] Validation for uploaded files
|
|
* [rev:113284] Added Form->enableSecurityToken() as a counterpart to the existing disableSecurityToken()
|
|
* [rev:113272] Added !SecurityToken to wrap CSRF protection via "SecurityID" request parameter
|
|
* [rev:112272] MySQLDatabase::renameField() no longer checks that the field exists in fieldList(). alterField() does no such check, so it should be consistent. Removing this should provide a small performance improvement as well
|
|
* [rev:111915] Added localisation for batch actions in javascript + translations
|
|
* [rev:111891] #4903 !MemberLoginForm field for "You are logged in as %s" message customisation (thanks walec51!)
|
|
* [rev:111887] #3775 Added getter to GD so you can retrieve the internal GD resource being used. Made setGD public so you can override the GD yourself as well
|
|
* [rev:111873] Show "Database Configuration" section of installer requirements for reference (collapsed by default)
|
|
* [rev:111868] MySQLDatabase::getVersion() now uses mysql_get_server_info() which has been supported since PHP 4. This gives us a better version than say "5.1", instead we now get something like "5.1.51"
|
|
* [rev:111850] Make use of mysql_get_server_info() when calling MSSQLDatabase::getVersion(), if there's a problem getting info this way, falls back to using query for VERSION() details
|
|
* [rev:111828] 6017 - Configurable help link
|
|
* [rev:111495] Making "sake" script more portable by using "/usr/bin/env" shebang instead of "/bin/bash" (fixes #6045, thanks sychan)
|
|
* [rev:111489] Added "module=" argument to !FullTestSuite (to support comma-separated module lists)
|
|
* [rev:111449] allow !PageCommentForm to store all users data, rather than hardcoding the fields
|
|
* [rev:111443] simple extend hook for !PageCommentForms. Temporary measure till #6053 is implemented
|
|
* [rev:111086] #6023 Shorten SSViewer cached template path for readability of the filenames, and also so Windows doesn't break on long paths
|
|
* [rev:111050] Added custom test listener for PHPUnit in order to call setUpOnce() and tearDownOnce() on !SapphireTest
|
|
* [rev:111048] Allowing to run single tests via phpunit through new test bootstrap XML file (e.g. "phpunit sapphire/tests/api/!RestfulServerTest.php" or "phpunit sapphire/tests/api")
|
|
* [rev:111045] Added !FullTestSuite.php, so that you can test by running "phpunit sapphire/tests/!FullTestSuite".
|
|
* [rev:111041] refactored runTests, using the new phpunit wrapper classes.
|
|
* [rev:111039] Created a phpunit wrapper class to ensure that Sapphire's test framework is capable of running unit tests, coverage report and retrieve clover-statistics for PHPUnit 3.4 and PHPUnit 3.5
|
|
|
|
|
|
### API Changes
|
|
|
|
* [rev:113282] Fixed various controllers to enforce CSRF protection through Form_!SecurityToken on GET actions that are not routed through Form->httpSubmission(): !AssetAdmin, CMSBatchActionHandler, CMSMain, !CommentTableField, !LeftAndMain, !MemberTableField, !PageComment, !PageComment_Controller
|
|
* [rev:113275] Added security token to !TableListField->Link() in order to include it in all URL actions automatically. This ensures that field actions bypassing Form->httpSubmission() still get CSRF protection
|
|
|
|
|
|
### Bugfixes
|
|
|
|
* [rev:113590] ErrorPage::requireDefaultRecords() case where no assets directory causes an fopen() error. Ensure assets directory is created before attempting to write error page files
|
|
* [rev:113419] Better checking of file validity (#6093) Thanks Pigeon
|
|
* [rev:113295] Ensure that !SearchForm searchEngine() call properly escapes the Relevance field for ANSI compliance
|
|
* [rev:113277] Clear static marking caches on Hierarchy->flushCache()
|
|
* [rev:113276] Fixed !ComplexTableField and !TableListField GET actions against CSRF attacks (with Form_!SecurityToken->checkRequest())
|
|
* [rev:113273] Using current controller for !MemberTableField constructor in Group->getCMSFields() instead of passing in a wrong instance (Group)
|
|
* [rev:113249] ModelViewer doesn't work due to minor bug introduced by making $_CLASS_MANIFEST keys lowercase (fixes #6144, thanks daniel.lindkvist)
|
|
* [rev:113247] Fixed month conversion in !DateField_View_JQuery::convert_iso_to_jquery_format() (fixes #6124, thanks mbren and natmchugh)
|
|
* [rev:113193] removed taiwans province of china
|
|
* [rev:113157] Add PHPUnit includes to !SapphireTest? class (can be loaded outside of !TestRunner? for static calls, in which case the PHPUnit autoloaders/includes aren't in place yet) (merged from r113156)
|
|
* [rev:113107] Use correct language code for jquery-ui date picker for en_US
|
|
* [rev:112961] Don't include web.config in the assets tracked in the File table.
|
|
* [rev:112288] Renamed !MySQLQuery::__destroy() renamed to __destruct() so that it is called properly after the object is destroyed
|
|
* [rev:112258] one more requirement switched to SSL
|
|
* [rev:111949] Ensure that \r carriage return characters get stripped out before setting content in HTMLValue::setContent(). DOMDocument will transform these into 
 entities, which is apparently XML spec, but not necessary for us as we're using HTML
|
|
* [rev:111932] #6089 Avoid javascript error when "Allow drag & drop reordering" enabled, and attempt to drag a file from one folder to another is performed
|
|
* [rev:111914] #6096 RSSFeed::feedContent() restores previous state of SSViewer::get_source_file_comments() after temporarily disabling it (thanks paradigmincarnate!)
|
|
* [rev:111898] Filesystem::removeFolder() did not remove files that ended with a "." when this is a valid file. Remove the regex and replace with specific case for "." and ".."
|
|
* [rev:111890] #6066 Form::__construct() should respect hasMethod on passed in Controller instance if it's available (thanks paradigmincarnate!)
|
|
* [rev:111889] #3910 Setting timezone parameter to !MySQLDatabase::__construct() should use $this->query() to be consistent
|
|
* [rev:111878] Ensure that windows-style newlines ("\r\n") don't get converted to their XML entity representation through DOMDocument in SS_HTMLValue->setContent()
|
|
* [rev:111843] More common defaults for en_US.xml used by Zend_!DateFormat (and !DateField/!DatetimeField), with less error prone numerical format replacing the Zend default of shortened month names (fixes #6071, thanks dalesaurus)
|
|
* [rev:111843] Correct locale mapping in !DateField_View_JQuery for "en_US" and "en_NZ"
|
|
* [rev:111842] #6055 !ErrorPage should always create static error page files when dev/build is called if they don't exist
|
|
* [rev:111841] RFC 2822 compliant validation of email adresses in !EmailField->jsValidation() and !EmailField->validate() (fixes #6067, thanks paradigmincarnate)
|
|
* [rev:111772] DB::connect() should not rely on $_SESSION existing, so we check isset() to supress any warnings of undefined indexes
|
|
* [rev:111494] Changing File->Filename property from arbitrary limitation on VARCHAR (255 characters) to TEXT (65k characters) to ensure the framework can handle deeply nested filesystem trees (fixes #6015, thanks muzdowski)
|
|
* [rev:111493] Moving folder after executing Folder::findOrMake will not set the Filenames properly. Invoking updateFilesystem() in File->onAfterWrite() instead of onBeforeWrite(), and avoid caching in FIle->getRelativePath() (fixes #5994 and #5937, thanks muzdowski)
|
|
* [rev:111492] Removing overloaded !TableField->sourceItems() method, which enables features of the underlying !TableListField implementation, such as pagination and source item caching (fixed #5965, thanks martijn)
|
|
* [rev:111464] Search didn't respect searchableClasses passed to !FulltextSearchable::enable()
|
|
* [rev:111452] added validation to the page comment form
|
|
* [rev:111255] ContentController::!SiteConfig() should look to the !SiteTree record so an alternate !SiteConfig is considered, if this method doesn't exist on the data record then fall back to the default !SiteConfig
|
|
* [rev:111202] Fixed quoting and GROUP BY statement in !ManyManyComplexTableField->getQuery() for Postgres compatibility
|
|
* [rev:111176] Force tidy to avoid wrapping long lines in CSSContentParser, it breaks our !FunctionalTest string assertions
|
|
* [rev:111126] TarballArchive::extractTo() uses an incorrectly spelled argument
|
|
* [rev:111097] Fixed !PhpSyntaxTest not to rely on relative folder references (broken due to chdir() changes in cli-script.php and bootstrap.php)
|
|
* [rev:111092] Fixed regression where coverage report request did not get passed through to runTests() in !TestRunner::all()
|
|
* [rev:111091] Fixed regression of dev/tests/all running a coverage report instead of just unit tests
|
|
* [rev:111049] Unset $default_session when using Session::clear_all()
|
|
* [rev:111044] Allow execution of a test without a current controller.
|
|
* [rev:111043] Don't require a current controller for Session::get/set/etc to work.
|
|
|
|
|
|
### Minor changes
|
|
|
|
* [rev:113450] Fixed output spelling mistake and formatting in !SapphireTest::delete_all_temp_dbs()
|
|
* [rev:113430] Fixed RSSFeedTest which should put test configuration code into setUp() and tearDown() methods. If the test fails halfway through, these will get called to clean up the state
|
|
* [rev:113360] Fixed regression from r113282 for changed !SecurityToken API in CMSMain->publishall() (fixes #6159)
|
|
* [rev:113281] Removed unused !SecurityAdmin->!MemberForm() and savemember() (see !MemberTableField)
|
|
* [rev:113280] Removed unused Security->addmember() (see !MemberTableField and !SecurityAdmin->addtogroup())
|
|
* [rev:113279] Removed unused !SecurityAdmin->removememberfromgroup() (see !MemberTableField)
|
|
* [rev:113278] Removed unused !MemberList templates (see !MemberTableField)
|
|
* [rev:113274] Using !SecurityToken in !ViewableData->getSecurityID()
|
|
* [rev:113248] Javascript translations in CMSMain_right.js (fixes #6142)
|
|
* [rev:113241] Documentation
|
|
* [rev:112982] updated typo in comment for Cache.
|
|
* [rev:112962] Fix to !SapphireInfo for git-svn checkouts.
|
|
* [rev:112961] Add documentation to File::$allowed_extensions explaining that there are config files to edit in assets/
|
|
* [rev:112321] Removed "In line of " text in CLI test reporter which did not work. Details are in the backtrace below anyway, so it's not required
|
|
* [rev:112278] Reverted regression in r112272
|
|
* [rev:112254] change the requirement's link to use current protocol (we don't want messages from browsers saying the page has unsecured content, when accessing the CMS over SSL)
|
|
* [rev:111950] Comment about HTMLValue::setContent() stripping out of carriage returns
|
|
* [rev:111903] #6083 !FileTest doesn't remove test folders and files created during test
|
|
* [rev:111899] Use Filesystem::removeFolder() in !FilesystemPublisherTest::tearDown() instead of specific code to handle this
|
|
* [rev:111898] Code syntax formatting of Filesystem::removeFolder()
|
|
* [rev:111888] Moved GD::set_default_quality() function to the top of the file to align with conventions
|
|
* [rev:111883] #6090 !FilesystemPublisherTest now stores temporary files in assets, which is writable, instead of the webroot which almost never has write permissions
|
|
* [rev:111875] Enable non-default language for tinyMCE, setting language in _config.php didn't work. Thanks for @christian
|
|
* [rev:111852] Revert r111850 to !MySQLDatabase::getVersion as version comparisons need to happen, and this will strip out non-numeric characters e.g. "ubuntu1" or "lenny4" which are prefixed on some Linux distros
|
|
* [rev:111851] dev/build now shows database name and version next to "Building database ..." text
|
|
* [rev:111844] Fixed regression from r111843 (i18nText, !MemberDatetimeFieldTest, !MemberTest)
|
|
* [rev:111843] Fixed form validation message in !DateField to include actual date format, rather than a hardcoded value
|
|
* [rev:111821] Change matchesRoughly threshold slightly in !DbDatetimeTest to allow for slower database server connections
|
|
* [rev:111789] Added !FulltextSearchable::get_searchable_classes() in order to introspect currently searchable classes, added !FulltextSearchableTest, added documentation
|
|
* [rev:111788] Fixed documentation in !CheckboxSetField (fixes #6068, thanks paradigmincarnate)
|
|
* [rev:111787] Fixed documentation in Datetime (fixes #6062, thanks nicolaas)
|
|
* [rev:111786] Fixed SS_Datetime references in !BrokenLinksReport and !CommentAdmin (fixes #6063, thanks nicolaas)
|
|
* [rev:111772] Code formatting tidy of DB::connect() function
|
|
* [rev:111748] CoreTest::testGetTempPathInProject() will try to create a temp dirs when running. !CoreTest::tearDown() will now remove these temp dirs when the test finishes
|
|
* [rev:111676] #5943 Debug::text() boolean values are amended with (bool) so they don't get confused with "true" or "false" which could be strings (thanks Pigeon!)
|
|
* [rev:111669] Unit test breaks if another module or project extends Folder
|
|
* [rev:111597] Updated language master file
|
|
* [rev:111497] Fixed indentation in !PageCommentInterface.js
|
|
* [rev:111496] Fixed SQL quoting bug in !FolderTest (caused by r111493)
|
|
* [rev:111454] removed debug
|
|
* [rev:111450] removed debug
|
|
* [rev:111262] Add translation correction for Czech and add Slovakian translation in cms and sapphire (js). Thanks to @Pike
|
|
* [rev:111224] Ensuring !SiteTreeAccess.js is properly minified in live mode
|
|
* [rev:111133] Code formatting in !FullTestSuite
|
|
* [rev:111123] Spelling corrections to Director comments
|
|
* [rev:111116] PHPUnit annotations for !PhpSyntaxTest
|
|
* [rev:111053] Removing !MemberImportFormTest, breaks PHPUnit test run, and doesnt have any assertions
|
|
* [rev:111052] Documentation for constants in Core.php
|
|
* [rev:111051] Don't use chdir(), it confuses the hell out of phpunit (e.g. directory_exists() and realpath() no longer work as expected)
|
|
* [rev:111047] Fixed SSViewerTest to initialize controller properly
|
|
* [rev:111046] Remove all session data in !TestSession that might've been set by the test harness (necessary for test runs through the phpunit binary)
|
|
* [rev:111042] added phpdoc to the new PHPUnitWrapper classes.
|
|
|
|
|
|
### Other
|
|
|
|
* [rev:111880] #4029 On the fly form validation works in Opera as well
|
|
* [rev:111879] Added doc for static help_link
|
|
* [rev:111452] Fixes #2782
|
|
* [rev:111040] API-CHANGE: remove include which is not required.
|
|
* [rev:111038] ENHACENEMENT: Change behaviour of the !MenufestBuilder to use spl_autoload_register instead of traditional __autoload.
|
|
|
|
<code>sscreatechangelog --version 2.4.3 --branch branches/2.4 --stopbranch tags/2.4.2</code> |