diff --git a/code/AssetAdmin.php b/code/AssetAdmin.php index 86077ea2..86e33e13 100755 --- a/code/AssetAdmin.php +++ b/code/AssetAdmin.php @@ -492,7 +492,10 @@ JS; /** * Add a new folder and return its details suitable for ajax. */ - public function addfolder() { + public function addfolder($request) { + // Protect against CSRF on destructive action + if(!SecurityToken::inst()->checkRequest($request)) return $this->httpError(400); + $parent = ($_REQUEST['ParentID'] && is_numeric($_REQUEST['ParentID'])) ? (int)$_REQUEST['ParentID'] : 0; $name = (isset($_REQUEST['Name'])) ? basename($_REQUEST['Name']) : _t('AssetAdmin.NEWFOLDER',"NewFolder"); @@ -536,7 +539,7 @@ JS; /** * Delete a folder */ - public function deletefolder() { + public function deletefolder($data, $ofmr) { $script = ''; $ids = split(' *, *', $_REQUEST['csvIDs']); $script = ''; @@ -567,7 +570,10 @@ JS; echo $script; } - public function removefile(){ + public function removefile($request){ + // Protect against CSRF on destructive action + if(!SecurityToken::inst()->checkRequest($request)) return $this->httpError(400); + if($fileID = $this->urlParams['ID']) { $file = DataObject::get_by_id('File', $fileID); // Delete the temp verions of this file in assets/_resampled @@ -612,7 +618,10 @@ JS; * Removes all unused thumbnails from the file store * and returns the status of the process to the user. */ - public function deleteunusedthumbnails() { + public function deleteunusedthumbnails($request) { + // Protect against CSRF on destructive action + if(!SecurityToken::inst()->checkRequest($request)) return $this->httpError(400); + $count = 0; $thumbnails = $this->getUnusedThumbnails(); diff --git a/code/CMSBatchActionHandler.php b/code/CMSBatchActionHandler.php index 81893ea2..c77ffc57 100644 --- a/code/CMSBatchActionHandler.php +++ b/code/CMSBatchActionHandler.php @@ -52,6 +52,9 @@ class CMSBatchActionHandler extends RequestHandler { Director::redirectBack(); return; } + + // Protect against CSRF on destructive action + if(!SecurityToken::inst()->checkRequest($request)) return $this->httpError(400); $actions = Object::get_static($this->class, 'batch_actions'); $actionClass = $actions[$request->param('BatchAction')]; diff --git a/code/CMSMain.php b/code/CMSMain.php index 4de384f5..7df9b7ea 100644 --- a/code/CMSMain.php +++ b/code/CMSMain.php @@ -436,7 +436,10 @@ JS; // Data saving handlers - public function addpage() { + public function addpage($data, $form) { + // Protect against CSRF on destructive action + if(!SecurityToken::inst()->checkRequest($this->request)) return $this->httpError(400); + $className = isset($_REQUEST['PageType']) ? $_REQUEST['PageType'] : "Page"; $parent = isset($_REQUEST['ParentID']) ? $_REQUEST['ParentID'] : 0; $suffix = isset($_REQUEST['Suffix']) ? "-" . $_REQUEST['Suffix'] : null; @@ -447,6 +450,7 @@ JS; } if(is_numeric($parent)) $parentObj = DataObject::get_by_id("SiteTree", $parent); + else $parentObj = null; if(!$parentObj || !$parentObj->ID) $parent = 0; if($parentObj && !$parentObj->canAddChildren()) return Security::permissionFailure($this); @@ -650,7 +654,7 @@ JS; /** * Roll a page back to a previous version */ - function rollback() { + function rollback($data, $form) { if(isset($_REQUEST['Version']) && (bool)$_REQUEST['Version']) { $record = $this->performRollback($_REQUEST['ID'], $_REQUEST['Version']); echo sprintf(_t('CMSMain.ROLLEDBACKVERSION',"Rolled back to version #%d. New version number is #%d"),$_REQUEST['Version'],$record->Version); @@ -660,7 +664,7 @@ JS; } } - function unpublish() { + function unpublish($data, $form) { $SQL_id = Convert::raw2sql($_REQUEST['ID']); $page = DataObject::get_by_id("SiteTree", $SQL_id); @@ -928,7 +932,10 @@ JS; return $this->batchactions()->batchActionList(); } - function buildbrokenlinks() { + function buildbrokenlinks($request) { + // Protect against CSRF on destructive action + if(!SecurityToken::inst()->checkRequest($request)) return $this->httpError(400); + if($this->urlParams['ID']) { $newPageSet[] = DataObject::get_by_id("Page", $this->urlParams['ID']); } else { @@ -1010,13 +1017,16 @@ JS; echo '

' . _t('CMSMain.TOTALPAGES',"Total pages: ") . "$count

"; } - function publishall() { + function publishall($request) { ini_set("memory_limit", -1); ini_set('max_execution_time', 0); $response = ""; if(isset($this->requestParams['confirm'])) { + // Protect against CSRF on destructive action + if(!SecurityToken::inst()->checkRequest($request)) return $this->httpError(400); + $start = 0; $pages = DataObject::get("SiteTree", "", "", "", "$start,30"); $count = 0; @@ -1041,14 +1051,16 @@ JS; $response .= sprintf(_t('CMSMain.PUBPAGES',"Done: Published %d pages"), $count); } else { + $token = SecurityToken::inst(); $response .= '

' . _t('CMSMain.PUBALLFUN','"Publish All" functionality') . '

' . _t('CMSMain.PUBALLFUN2', 'Pressing this button will do the equivalent of going to every page and pressing "publish". It\'s intended to be used after there have been massive edits of the content, such as when the site was first built.') . '

-
'; + . _t('CMSMain.PUBALLCONFIRM',"Please publish every page in the site, copying content stage to live",PR_LOW,'Confirmation button') .'" />' + . $token->getFormField()->FieldHolder() . + ''; } return $response; @@ -1057,7 +1069,7 @@ JS; /** * Restore a completely deleted page from the SiteTree_versions table. */ - function restore() { + function restore($data, $form) { if(($id = $_REQUEST['ID']) && is_numeric($id)) { $restoredPage = Versioned::get_latest_version("SiteTree", $id); if($restoredPage) { @@ -1077,7 +1089,10 @@ JS; } } - function duplicate() { + function duplicate($request) { + // Protect against CSRF on destructive action + if(!SecurityToken::inst()->checkRequest($request)) return $this->httpError(400); + if(($id = $this->urlParams['ID']) && is_numeric($id)) { $page = DataObject::get_by_id("SiteTree", $id); if($page && !$page->canEdit()) return Security::permissionFailure($this); @@ -1096,7 +1111,10 @@ JS; } } - function duplicatewithchildren() { + function duplicatewithchildren($request) { + // Protect against CSRF on destructive action + if(!SecurityToken::inst()->checkRequest($request)) return $this->httpError(400); + if(($id = $this->urlParams['ID']) && is_numeric($id)) { $page = DataObject::get_by_id("SiteTree", $id); if($page && !$page->canEdit()) return Security::permissionFailure($this); @@ -1114,7 +1132,10 @@ JS; /** * Create a new translation from an existing item, switch to this language and reload the tree. */ - function createtranslation () { + function createtranslation($request) { + // Protect against CSRF on destructive action + if(!SecurityToken::inst()->checkRequest($request)) return $this->httpError(400); + $langCode = Convert::raw2sql($_REQUEST['newlang']); $originalLangID = (int)$_REQUEST['ID']; diff --git a/code/CommentTableField.php b/code/CommentTableField.php index a9a10902..84a70b12 100644 --- a/code/CommentTableField.php +++ b/code/CommentTableField.php @@ -80,6 +80,10 @@ class CommentTableField extends ComplexTableField { return $fieldContainer->FieldHolder(); } + + function handleItem($request) { + return new CommentTableField_ItemRequest($this, $request->param('ID')); + } } /** @@ -88,6 +92,7 @@ class CommentTableField extends ComplexTableField { * @subpackage comments */ class CommentTableField_Item extends ComplexTableField_Item { + function HasSpamButton() { return $this->parent()->HasSpamButton(); } @@ -99,6 +104,51 @@ class CommentTableField_Item extends ComplexTableField_Item { function HasHamButton() { return $this->parent()->HasHamButton(); } + + /** + * @return String + */ + function SpamLink() { + return Controller::join_links($this->Link(), "pagecommentaction", 'reportspam', $this->ID); + } + + /** + * @return String + */ + function HamLink() { + return Controller::join_links($this->Link(), "pagecommentaction", 'reportham', $this->ID); + } + + /** + * @return String + */ + function ApproveLink() { + return Controller::join_links($this->Link(), "pagecommentaction", 'approve', $this->ID); + } } +/** + * @package cms + * @subpackage comments + */ +class CommentTableField_ItemRequest extends ComplexTableField_ItemRequest { + + static $url_handlers = array( + 'pagecommentaction/$Action/$ID' => 'handlePageCommentAction', + ); + + /** + * @param SS_HTTPRequest $request + * @return SS_HTTPResponse + */ + function handlePageCommentAction($request) { + $action = $request->param('Action'); + $whitelist = array('approve', 'reportspam', 'reportham'); + if(!in_array($action, $whitelist)) $this->httpError(403); + + $c = new PageComment_Controller($request); + $c->init(); + return $c->$action($request); + } +} ?> \ No newline at end of file diff --git a/code/LeftAndMain.php b/code/LeftAndMain.php index 6986572d..9777c0a3 100644 --- a/code/LeftAndMain.php +++ b/code/LeftAndMain.php @@ -772,7 +772,10 @@ JS; /** * Ajax handler for updating the parent of a tree node */ - public function ajaxupdateparent() { + public function ajaxupdateparent($request) { + // Protect against CSRF on destructive action + if(!SecurityToken::inst()->checkRequest($request)) return $this->httpError(400); + $id = $_REQUEST['ID']; $parentID = $_REQUEST['ParentID']; if($parentID == 'root'){ @@ -816,7 +819,10 @@ JS; * $_GET[ID]: An array of node ids in the correct order * $_GET[MovedNodeID]: The node that actually got moved */ - public function ajaxupdatesort() { + public function ajaxupdatesort($request) { + // Protect against CSRF on destructive action + if(!SecurityToken::inst()->checkRequest($request)) return $this->httpError(400); + $className = $this->stat('tree_class'); $counter = 0; $js = ''; @@ -863,7 +869,10 @@ JS; /** * Delete a number of items */ - public function deleteitems() { + public function deleteitems($request) { + // Protect against CSRF on destructive action + if(!SecurityToken::inst()->checkRequest($request)) return $this->httpError(400); + $ids = split(' *, *', $_REQUEST['csvIDs']); $script = "st = \$('sitetree'); \n"; diff --git a/code/MemberTableField.php b/code/MemberTableField.php index f1617d6f..4c19e553 100755 --- a/code/MemberTableField.php +++ b/code/MemberTableField.php @@ -139,7 +139,7 @@ class MemberTableField extends ComplexTableField { } function AddLink() { - return $this->Link() . '/add'; + return Controller::join_links($this->Link(), 'add'); } function SearchForm() { @@ -167,6 +167,10 @@ class MemberTableField extends ComplexTableField { * Add existing member to group rather than creating a new member */ function addtogroup() { + // Protect against CSRF on destructive action + $token = $this->getForm()->getSecurityToken(); + if(!$token->checkRequest($this->controller->getRequest())) return $this->httpError(400); + $data = $_REQUEST; unset($data['ID']); $ctfID = isset($data['ctf']) ? $data['ctf']['ID'] : null; @@ -204,6 +208,11 @@ class MemberTableField extends ComplexTableField { * Remove member from group rather than from the database */ function delete() { + // Protect against CSRF on destructive action + $token = $this->getForm()->getSecurityToken(); + // TODO Not sure how this is called, using $_REQUEST to be on the safe side + if(!$token->check($_REQUEST['SecurityID'])) return $this->httpError(400); + $groupID = Convert::raw2sql($_REQUEST['ctf']['ID']); $memberID = Convert::raw2sql($_REQUEST['ctf']['childID']); if(is_numeric($groupID) && is_numeric($memberID)) { @@ -391,7 +400,11 @@ class MemberTableField_ItemRequest extends ComplexTableField_ItemRequest { /** * Deleting an item from a member table field should just remove that member from the group */ - function delete() { + function delete($request) { + // Protect against CSRF on destructive action + $token = $this->ctf->getForm()->getSecurityToken(); + if(!$token->checkRequest($request)) return $this->httpError('400'); + if($this->ctf->Can('delete') !== true) { return false; } @@ -402,4 +415,4 @@ class MemberTableField_ItemRequest extends ComplexTableField_ItemRequest { } -?> +?> \ No newline at end of file diff --git a/code/SecurityAdmin.php b/code/SecurityAdmin.php index 96cc5b96..9eaf4c3d 100644 --- a/code/SecurityAdmin.php +++ b/code/SecurityAdmin.php @@ -152,7 +152,12 @@ class SecurityAdmin extends LeftAndMain implements PermissionProvider { return $siteTree; } - public function addgroup() { + public function addgroup($request) { + // Protect against CSRF on destructive action + if(!SecurityToken::inst()->checkRequest($request)) return $this->httpError(400); + + if(!singleton($this->stat('tree_class'))->canCreate()) return Security::permissionFailure($this); + $newGroup = Object::create($this->stat('tree_class')); $newGroup->Title = _t('SecurityAdmin.NEWGROUP',"New Group"); $newGroup->Code = "new-group"; diff --git a/code/sitefeatures/PageComment.php b/code/sitefeatures/PageComment.php index cd7657f9..80ffec81 100755 --- a/code/sitefeatures/PageComment.php +++ b/code/sitefeatures/PageComment.php @@ -42,7 +42,7 @@ class PageComment extends DataObject { * @return string link to this comment. */ function Link() { - return $this->Parent()->Link() . '#PageComment_'. $this->ID; + return Controller::join_links($this->Parent()->Link(), '#PageComment_'. $this->ID); } function ParsedBBCode(){ @@ -51,19 +51,39 @@ class PageComment extends DataObject { } function DeleteLink() { - return (Permission::check('CMS_ACCESS_CMSMain')) ? "PageComment_Controller/deletecomment/$this->ID" : false; + if(Permission::check('CMS_ACCESS_CMSMain')) { + $token = SecurityToken::inst(); + return $token->addToUrl("PageComment_Controller/deletecomment/$this->ID"); + } else { + return false; + } } function SpamLink() { - return (Permission::check('CMS_ACCESS_CMSMain') && !$this->IsSpam) ? "PageComment_Controller/reportspam/$this->ID" : false; + if(Permission::check('CMS_ACCESS_CMSMain') && !$this->IsSpam) { + $token = SecurityToken::inst(); + return $token->addToUrl("PageComment_Controller/reportspam/$this->ID"); + } else { + return false; + } } function HamLink() { - return (Permission::check('CMS_ACCESS_CMSMain') && $this->IsSpam) ? "PageComment_Controller/reportham/$this->ID" : false; + if(Permission::check('CMS_ACCESS_CMSMain') && $this->IsSpam) { + $token = SecurityToken::inst(); + return $token->addToUrl("PageComment_Controller/reportham/$this->ID"); + } else { + return false; + } } function ApproveLink() { - return (Permission::check('CMS_ACCESS_CMSMain') && $this->NeedsModeration) ? "PageComment_Controller/approve/$this->ID" : false; + if(Permission::check('CMS_ACCESS_CMSMain') && $this->NeedsModeration) { + $token = SecurityToken::inst(); + return $token->addToUrl("PageComment_Controller/approve/$this->ID"); + } else { + return false; + } } function SpamClass() { @@ -161,9 +181,13 @@ class PageComment_Controller extends Controller { $rss->outputToBrowser(); } - function deletecomment() { + function deletecomment($request) { + // Protect against CSRF on destructive action + $token = SecurityToken::inst(); + if(!$token->checkRequest($request)) return $this->httpError(400); + if(Permission::check('CMS_ACCESS_CMSMain')) { - $comment = DataObject::get_by_id("PageComment", $this->urlParams['ID']); + $comment = DataObject::get_by_id("PageComment", $request->param('ID')); if($comment) { $comment->delete(); } @@ -176,27 +200,34 @@ class PageComment_Controller extends Controller { } } - function approve() { + function approve($request) { + // Protect against CSRF on destructive action + $token = SecurityToken::inst(); + if(!$token->checkRequest($request)) return $this->httpError(400); + if(Permission::check('CMS_ACCESS_CMSMain')) { - $comment = DataObject::get_by_id("PageComment", $this->urlParams['ID']); - + $comment = DataObject::get_by_id("PageComment", $request->param('ID')); if($comment) { $comment->NeedsModeration = false; $comment->write(); - + // @todo Report to spamprotecter this is true - + if(Director::is_ajax()) { echo $comment->renderWith('PageCommentInterface_singlecomment'); } else { Director::redirectBack(); } } - } + } } - function reportspam() { - $comment = DataObject::get_by_id("PageComment", $this->urlParams['ID']); + function reportspam($request) { + // Protect against CSRF on destructive action + $token = SecurityToken::inst(); + if(!$token->checkRequest($request)) return $this->httpError(400); + + $comment = DataObject::get_by_id("PageComment", $request->param('ID')); if($comment) { // check they have access if(Permission::check('CMS_ACCESS_CMSMain')) { @@ -236,8 +267,12 @@ class PageComment_Controller extends Controller { /** * Report a Spam Comment as valid comment (not spam) */ - function reportham() { - $comment = DataObject::get_by_id("PageComment", $this->urlParams['ID']); + function reportham($request) { + // Protect against CSRF on destructive action + $token = SecurityToken::inst(); + if(!$token->checkRequest($request)) return $this->httpError(400); + + $comment = DataObject::get_by_id("PageComment", $request->param('ID')); if($comment) { if(Permission::check('CMS_ACCESS_CMSMain')) { diff --git a/javascript/AssetAdmin.js b/javascript/AssetAdmin.js index 9fa8a8ea..2d6514e8 100755 --- a/javascript/AssetAdmin.js +++ b/javascript/AssetAdmin.js @@ -481,7 +481,8 @@ Behaviour.register({ method: 'get', onSuccess: function(t) { eval(t.responseText); - } + }, + parameters: {"SecurityID": ($('SecurityID')) ? $('SecurityID').value : null} }; new Ajax.Request('admin/assets/deleteunusedthumbnails',options); } diff --git a/javascript/LeftAndMain_left.js b/javascript/LeftAndMain_left.js index cb86bdb1..8c7f8d84 100755 --- a/javascript/LeftAndMain_left.js +++ b/javascript/LeftAndMain_left.js @@ -249,8 +249,10 @@ TreeNodeAPI.prototype = { if(this.parentTreeNode && this.parentTreeNode.getIdx) { parentClause = "&parentID=" + this.parentTreeNode.getIdx(); } + var token = $$('input[name=SecurityID]')[0].value; + var url = baseHref() + 'admin/duplicate/' + this.getIdx() + '?ajax=1' + parentClause + '&SecurityID=' + token; - new Ajax.Request(baseHref() + 'admin/duplicate/' + this.getIdx() + '?ajax=1' + parentClause, { + new Ajax.Request(url, { method : 'get', onSuccess : Ajax.Evaluator, onFailure : function(response) { @@ -259,7 +261,9 @@ TreeNodeAPI.prototype = { }); }, duplicatePageWithChildren: function() { - new Ajax.Request(baseHref() + 'admin/duplicatewithchildren/' + this.getIdx() + '?ajax=1', { + var token = $$('input[name=SecurityID]')[0].value; + var url = baseHref() + 'admin/duplicatewithchildren/' + this.getIdx() + '?ajax=1&SecurityID=' + token; + new Ajax.Request(url, { method : 'get', onSuccess : Ajax.Evaluator, onFailure : function(response) { @@ -391,9 +395,10 @@ SiteTreeNode.prototype = { if($('Form_EditForm').elements.ID) currentlyOpenPageID = $('Form_EditForm').elements.ID.value; statusMessage(ss.i18n._t('CMSMAIN.SAVING'), '', true); + var token = $$('input[name=SecurityID]')[0].value; new Ajax.Request(SiteTreeHandlers.parentChanged_url, { method : 'post', - postBody : 'ID=' + node.getIdx() + '&ParentID=' + newParent.getIdx() + '&CurrentlyOpenPageID=' + currentlyOpenPageID, + postBody : 'ID=' + node.getIdx() + '&ParentID=' + newParent.getIdx() + '&CurrentlyOpenPageID=' + currentlyOpenPageID + '&SecurityID=' + token, onSuccess : Ajax.Evaluator, onFailure : function(response) { errorMessage('error saving parent', response); @@ -434,9 +439,10 @@ SiteTreeNode.prototype = { if($('Form_EditForm').elements.ID) currentlyOpenPageID = $('Form_EditForm').elements.ID.value; if(parts) { + var token = $$('input[name=SecurityID]')[0].value; new Ajax.Request(SiteTreeHandlers.orderChanged_url, { method : 'post', - postBody : parts.join('&') + '&CurrentlyOpenPageID=' + currentlyOpenPageID, + postBody : parts.join('&') + '&CurrentlyOpenPageID=' + currentlyOpenPageID + '&SecurityID=' + token, /*onSuccess : function(response) { // statusMessage(response.responseText, 'good'); },*/ diff --git a/javascript/TranslationTab.js b/javascript/TranslationTab.js index 3aff2269..a1aecfdb 100755 --- a/javascript/TranslationTab.js +++ b/javascript/TranslationTab.js @@ -9,6 +9,7 @@ Behaviour.register({ var url = baseHref() + 'admin/' + this.name.substring(7) + '?ID=' + $('Form_EditForm_ID').value + '&newlang=' + $('Form_EditForm_NewTransLang').value + '&ajax=1'; url += "&locale=" + $('Form_EditForm_Locale').value; + url += "&SecurityID=" + $$('input[name=SecurityID]')[0].value; new Ajax.Request( url, { onSuccess: Ajax.Evaluator, diff --git a/templates/Includes/AssetAdmin_left.ss b/templates/Includes/AssetAdmin_left.ss index bebe4f9c..88f125d0 100755 --- a/templates/Includes/AssetAdmin_left.ss +++ b/templates/Includes/AssetAdmin_left.ss @@ -8,6 +8,7 @@ diff --git a/templates/Includes/AssetTableField.ss b/templates/Includes/AssetTableField.ss index 2d0f04dc..29a93da4 100755 --- a/templates/Includes/AssetTableField.ss +++ b/templates/Includes/AssetTableField.ss @@ -40,7 +40,7 @@ <% end_if %> <% if Can(delete) %> - <% _t('DELFILE', 'Delete this file') %> + <% _t('DELFILE', 'Delete this file') %> <% end_if %> diff --git a/templates/Includes/CMSMain_left.ss b/templates/Includes/CMSMain_left.ss index ad20ad90..9da19a11 100755 --- a/templates/Includes/CMSMain_left.ss +++ b/templates/Includes/CMSMain_left.ss @@ -70,7 +70,7 @@

<% _t('SELECTPAGESACTIONS','Select the pages that you want to change & then click an action:') %>

- +
+ @@ -15,6 +16,7 @@

<% _t('SELECT','Select the pages that you want to delete and then click the button below') %>

+ diff --git a/tests/CMSMainTest.php b/tests/CMSMainTest.php index 978ba205..dfdd610b 100644 --- a/tests/CMSMainTest.php +++ b/tests/CMSMainTest.php @@ -7,16 +7,24 @@ class CMSMainTest extends FunctionalTest { * @todo Test the results of a publication better */ function testPublish() { + $page1 = $this->objFromFixture('Page', "page1"); + $page2 = $this->objFromFixture('Page', "page2"); $this->session()->inst_set('loggedInAs', $this->idFromFixture('Member', 'admin')); - $response = Director::test("admin/cms/publishall", array('confirm' => 1), $this->session()); + $response = $this->get("admin/cms/publishall?confirm=1"); $this->assertContains( sprintf(_t('CMSMain.PUBPAGES',"Done: Published %d pages"), 5), $response->getBody() ); - $response = Director::test("admin/cms/batchactions/publish", array('csvIDs' => '1,2', 'ajax' => 1), $this->session()); + $response = $this->post( + "admin/cms/batchactions/publish", + array( + 'csvIDs' => implode(',', array($page1->ID, $page2->ID)), + 'ajax' => 1 + ) + ); $this->assertContains('setNodeTitle(1, \'Page 1\');', $response->getBody()); $this->assertContains('setNodeTitle(2, \'Page 2\');', $response->getBody()); @@ -126,4 +134,5 @@ class CMSMainTest extends FunctionalTest { $this->assertEquals('5', $newPage->ParentID); } + } \ No newline at end of file