Fixed a security issue with the CSRF protection being avoidable using some site admin functionality

- as discovered and reported by Egidio Romano of Minded Security

- Lacking checks of HTTP methods in some functionality could lead to a site admin uploading and executing malicious scripts

- Tightened HTTP method verification across the board for actions that modify data
- Turned some administrative tasks to POST only actions
pull/727/head
iglocska 2015-11-13 23:57:03 +01:00
parent a380458d2e
commit afdcc1af0c
9 changed files with 34 additions and 51 deletions

View File

@ -362,7 +362,7 @@ class AppController extends Controller {
}
public function generateCount() {
if (!self::_isSiteAdmin()) throw new NotFoundException();
if (!self::_isSiteAdmin() || !$this->request->is('post')) throw new NotFoundException();
// do one SQL query with the counts
// loop over events, update in db
$this->loadModel('Attribute');
@ -382,7 +382,7 @@ class AppController extends Controller {
}
public function pruneDuplicateUUIDs() {
if (!$this->_isSiteAdmin()) throw new MethodNotAllowedException();
if (!$this->_isSiteAdmin() || !$this->request->is('post')) throw new MethodNotAllowedException();
$this->LoadModel('Attribute');
$duplicates = $this->Attribute->find('all', array(
'fields' => array('Attribute.uuid', 'count(*) as occurance'),
@ -409,7 +409,7 @@ class AppController extends Controller {
}
public function removeDuplicateEvents() {
if (!$this->_isSiteAdmin()) throw new MethodNotAllowedException();
if (!$this->_isSiteAdmin() || !$this->request->is('post')) throw new MethodNotAllowedException();
$this->LoadModel('Event');
$duplicates = $this->Event->find('all', array(
'fields' => array('Event.uuid', 'count(*) as occurance'),
@ -445,7 +445,7 @@ class AppController extends Controller {
}
public function updateDatabase($command) {
if (!$this->_isSiteAdmin()) throw new MethodNotAllowedException();
if (!$this->_isSiteAdmin() || !$this->request->is('post')) throw new MethodNotAllowedException();
$this->loadModel('Server');
$this->Server->updateDatabase($command);
$this->Session->setFlash('Done.');

View File

@ -923,7 +923,7 @@ class AttributesController extends AppController {
}
public function deleteSelected($id) {
if (!$this->request->is('post') && !$this->request->is('ajax')) {
if (!$this->request->is('post') || !$this->request->is('ajax')) {
//if (!$this->request->is('post')) {
throw new MethodNotAllowedException();
}
@ -1840,7 +1840,7 @@ class AttributesController extends AppController {
}
public function generateCorrelation() {
if (!self::_isSiteAdmin()) throw new NotFoundException();
if (!self::_isSiteAdmin() || !$this->request->is('post')) throw new NotFoundException();
if (!Configure::read('MISP.background_jobs')) {
$k = $this->Attribute->generateCorrelation();
$this->Session->setFlash(__('All done. ' . $k . ' attributes processed.'));
@ -2217,7 +2217,7 @@ class AttributesController extends AppController {
}
public function pruneOrphanedAttributes() {
if (!$this->_isSiteAdmin()) throw new MethodNotAllowedException('You are not authorised to do that.');
if (!$this->_isSiteAdmin() || !$this->request->is('post')) throw new MethodNotAllowedException('You are not authorised to do that.');
$events = array_keys($this->Attribute->Event->find('list'));
$orphans = $this->Attribute->find('list', array('conditions' => array('Attribute.event_id !=' => $events)));
if (count($orphans) > 0) $this->Attribute->deleteAll(array('Attribute.event_id !=' => $events), false, true);

View File

@ -2621,7 +2621,7 @@ class EventsController extends AppController {
}
public function create_dummy_event() {
if (!$this->_isSiteAdmin()) throw new MethodNotAllowedException('You don\'t have the privileges to access this.');
if (!$this->_isSiteAdmin() || !$this->request->is('post')) throw new MethodNotAllowedException('You don\'t have the privileges to access this.');
$date = new DateTime();
$data['Event']['info'] = 'Test event showing every category-type combination';
$data['Event']['date'] = '2013-10-09';
@ -2689,7 +2689,7 @@ class EventsController extends AppController {
// for load testing, it's slow, execution time is set at 1 hour maximum
public function create_massive_dummy_events() {
if (!$this->_isSiteAdmin()) throw new MethodNotAllowedException('You don\'t have the privileges to access this.');
if (!$this->_isSiteAdmin() || !$this->request->is('post')) throw new MethodNotAllowedException('You don\'t have the privileges to access this.');
ini_set('max_execution_time', 3600);
$this->Event->Behaviors->unload('SysLogLogable.SysLogLogable');
$date = new DateTime();
@ -2797,21 +2797,6 @@ class EventsController extends AppController {
$this->set('count', $count);
}
public function generateLocked() {
if (!self::_isSiteAdmin()) throw new NotFoundException();
$toBeUpdated = $this->Event->generateLocked();
$this->Session->setFlash('Events updated, '. $toBeUpdated . ' record(s) altered.');
$this->redirect(array('controller' => 'pages', 'action' => 'display', 'administration'));
}
public function generateThreatLevelFromRisk() {
if (!self::_isSiteAdmin()) throw new NotFoundException();
$updated = $this->Event->generateThreatLevelFromRisk();
$this->Session->setFlash('Events updated, '. $updated . ' record(s) altered.');
$this->redirect(array('controller' => 'pages', 'action' => 'display', 'administration'));
}
public function addTag($id = false, $tag_id = false) {
if (!$this->request->is('post')) {
return new CakeResponse(array('body'=> json_encode(array('saved' => false, 'errors' => 'You don\'t have permission to do that.')), 'status'=>200));

View File

@ -194,7 +194,7 @@ class RegexpController extends AppController {
*
*/
public function admin_clean() {
if(!$this->_isSiteAdmin()) $this->redirect(array('controller' => 'regexp', 'action' => 'index', 'admin' => false));
if(!$this->_isSiteAdmin() || !$this->request->is('post')) throw new MethodNotAllowedException('This action is only accessible via a POST request.');
$allRegexp = $this->Regexp->find('all');
$deletable = array();
$modifications = 0;

View File

@ -260,7 +260,7 @@ class ServersController extends AppController {
}
}
public function __saveCert($server, $id) {
private function __saveCert($server, $id) {
$ext = '';
App::uses('File', 'Utility');
App::uses('Folder', 'Utility');
@ -424,7 +424,7 @@ class ServersController extends AppController {
}
public function startWorker($type) {
if (!$this->_isSiteAdmin() || !$this->request->is('Post')) throw new MethodNotAllowedException();
if (!$this->_isSiteAdmin() || !$this->request->is('post')) throw new MethodNotAllowedException();
$validTypes = array('default', 'email', 'scheduler', 'cache');
if (!in_array($type, $validTypes)) throw new MethodNotAllowedException('Invalid worker type.');
if ($type != 'scheduler') shell_exec(APP . 'Console' . DS . 'cake ' . DS . 'CakeResque.CakeResque start --interval 5 --queue ' . $type .' > /dev/null 2>&1 &');
@ -433,7 +433,7 @@ class ServersController extends AppController {
}
public function stopWorker($pid) {
if (!$this->_isSiteAdmin() || !$this->request->is('Post')) throw new MethodNotAllowedException();
if (!$this->_isSiteAdmin() || !$this->request->is('post')) throw new MethodNotAllowedException();
$this->Server->killWorker($pid, $this->Auth->user());
$this->redirect('/servers/serverSettings/workers');
}
@ -597,7 +597,7 @@ class ServersController extends AppController {
}
public function uploadFile($type) {
if (!$this->_isSiteAdmin()) throw new MethodNotAllowedException();
if (!$this->_isSiteAdmin() || !$this->request->is('post')) throw new MethodNotAllowedException();
$validItems = $this->Server->getFileRules();
// Check if there were problems with the file upload

View File

@ -1099,7 +1099,7 @@ class ShadowAttributesController extends AppController {
}
public function discardSelected($id) {
if (!$this->request->is('post') && !$this->request->is('ajax')) throw new MethodNotAllowedException();
if (!$this->request->is('post') || !$this->request->is('ajax')) throw new MethodNotAllowedException();
// get a json object with a list of proposal IDs to be discarded
// check each of them and return a json object with the successful discards and the failed ones.
@ -1135,7 +1135,7 @@ class ShadowAttributesController extends AppController {
}
public function acceptSelected($id) {
if (!$this->request->is('post') && !$this->request->is('ajax')) throw new MethodNotAllowedException();
if (!$this->request->is('post') || !$this->request->is('ajax')) throw new MethodNotAllowedException();
// get a json object with a list of proposal IDs to be accepted
// check each of them and return a json object with the successful accepts and the failed ones.

View File

@ -116,7 +116,7 @@ class UsersController extends AppController {
$this->request->data = $this->User->data;
}
// XXX ACL roles
$this->extraLog("change_pw");
$this->__extralog("change_pw");
$roles = $this->User->Role->find('list');
$this->set(compact('roles'));
}
@ -406,13 +406,13 @@ class UsersController extends AppController {
foreach (array_keys($this->request->data['User']) as $field) {
if($field != 'password') array_push($fields, $field);
}
// TODO Audit, extraLog, fields get orig
// TODO Audit, __extralog, fields get orig
$fieldsOldValues = array();
foreach ($fields as $field) {
if($field != 'confirm_password') array_push($fieldsOldValues, $this->User->field($field));
else array_push($fieldsOldValues, $this->User->field('password'));
}
// TODO Audit, extraLog, fields get orig END
// TODO Audit, __extralog, fields get orig END
if ("" != $this->request->data['User']['password'])
$fields[] = 'password';
$fields[] = 'role_id';
@ -425,7 +425,7 @@ class UsersController extends AppController {
}
}
if ($this->User->save($this->request->data, true, $fields)) {
// TODO Audit, extraLog, fields compare
// TODO Audit, __extralog, fields compare
// newValues to array
$fieldsNewValues = array();
foreach ($fields as $field) {
@ -455,8 +455,8 @@ class UsersController extends AppController {
$c++;
}
$fieldsResultStr = substr($fieldsResultStr, 2);
$this->extraLog("edit", "user", $fieldsResultStr); // TODO Audit, check: modify User
// TODO Audit, extraLog, fields compare END
$this->__extralog("edit", "user", $fieldsResultStr); // TODO Audit, check: modify User
// TODO Audit, __extralog, fields compare END
$this->Session->setFlash(__('The user has been saved'));
$this->_refreshAuth(); // in case we modify ourselves
$this->redirect(array('action' => 'index'));
@ -495,7 +495,7 @@ class UsersController extends AppController {
throw new NotFoundException(__('Invalid user'));
}
if ($this->User->delete()) {
$this->extraLog("delete", $fieldsDescrStr, ''); // TODO Audit, check: modify User
$this->__extralog("delete", $fieldsDescrStr, ''); // TODO Audit, check: modify User
$this->Session->setFlash(__('User deleted'));
$this->redirect(array('action' => 'index'));
}
@ -505,7 +505,7 @@ class UsersController extends AppController {
public function login() {
if ($this->Auth->login()) {
$this->extraLog("login"); // TODO Audit, extraLog, check: customLog i.s.o. extraLog, no auth user?: $this->User->customLog('login', $this->Auth->user('id'), array('title' => '','user_id' => $this->Auth->user('id'),'email' => $this->Auth->user('email'),'org' => 'IN2'));
$this->__extralog("login"); // TODO Audit, __extralog, check: customLog i.s.o. __extralog, no auth user?: $this->User->customLog('login', $this->Auth->user('id'), array('title' => '','user_id' => $this->Auth->user('id'),'email' => $this->Auth->user('email'),'org' => 'IN2'));
// TODO removed the auto redirect for now, due to security concerns - will look more into this
// $this->redirect($this->Auth->redirectUrl());
$this->redirect(array('controller' => 'events', 'action' => 'index'));
@ -570,7 +570,7 @@ class UsersController extends AppController {
public function logout() {
if ($this->Session->check('Auth.User')) { // TODO session, user is logged in, so ..
$this->extraLog("logout"); // TODO Audit, extraLog, check: customLog i.s.o. extraLog, $this->User->customLog('logout', $this->Auth->user('id'), array());
$this->__extralog("logout"); // TODO Audit, __extralog, check: customLog i.s.o. __extralog, $this->User->customLog('logout', $this->Auth->user('id'), array());
}
$this->Session->setFlash(__('Good-Bye'));
$this->redirect($this->Auth->logout());
@ -730,7 +730,7 @@ class UsersController extends AppController {
return $this->response;
}
public function extraLog($action = null, $description = null, $fieldsResult = null) { // TODO move audit to AuditsController?
private function __extralog($action = null, $description = null, $fieldsResult = null) { // TODO move audit to AuditsController?
// new data
$userId = $this->Auth->user('id');
$model = 'User';

View File

@ -79,7 +79,7 @@
<li <?php if ($menuItem === 'index') echo 'class="active"';?>><?php echo $this->Html->link('List Regexp', array('admin' => $isSiteAdmin, 'action' => 'index'));?></li>
<?php if ($isSiteAdmin): ?>
<li <?php if ($menuItem === 'add') echo 'class="active"';?>><?php echo $this->Html->link('New Regexp', array('admin' => true, 'action' => 'add'));?></li>
<li><?php echo $this->Html->link('Perform on existing', array('admin' => true, 'action' => 'clean'));?></li>
<li><?php echo $this->Form->postLink('Perform on existing', array('admin' => true, 'action' => 'clean'));?></li>
<?php endif;
if ($menuItem == 'edit'):?>
<li class="divider"></li>

View File

@ -11,16 +11,14 @@ if (!$isSiteAdmin) exit();
<ul>
<li><a href="/events/reportValidationIssuesEvents">reportValidationIssuesEvents</a></li>
<li><a href="/attributes/reportValidationIssuesAttributes">reportValidationIssuesAttributes</a></li>
<li><a href="/events/generateCount">generateCount</a> (Events need to have no validation issues)</li>
<li><a href="/attributes/generateCorrelation">generateCorrelation</a></li>
<li><a href="/events/generateLocked">generateLocked</a> (This is for upgrading to hotfix 2.1.8 or later, all events that were created by an organisation that doesn't have users on this instance, or only has a single sync user will have their locked setting set to 1)</li>
<li><?php echo $this->Form->postLink('generateCount', '/events/generateCount');?> (Events need to have no validation issues)</li>
<li><?php echo $this->Form->postLink('generateCorrelation', '/attributes/generateCorrelation');?></li>
<li><a href="/users/verifyGPG">Verify GPG keys</a> (Check whether every user's GPG key is usable)</li>
<li><a href="/events/generateThreatLevelFromRisk">Upgrade Risk to Threat Level</a> (As of version 2.2 the risk field is replaced by Threat Level. This script will convert all values in risk to threat level.)</li>
<li><a href="/servers/updateDatabase/extendServerOrganizationLength">Extend Organization length</a> (Hotfix 2.3.57: Increase the max length of the organization field when adding a new server connection.)</li>
<li><a href="/servers/updateDatabase/convertLogFieldsToText">Convert log fields to text</a> (Hotfix 2.3.78: Some of the log fields that were varchar(255) ended up truncating the data. This function will change them to "text")</li>
<li><a href="/servers/pruneDuplicateUUIDs">Fix duplicate UUIDs</a> (Hotfix 2.3.107: it was previously possible to get duplicate attribute UUIDs in the database, this script will remove all duplicates and ensure that duplicates will not be entered into the database in the future.)</li>
<li><a href="/servers/removeDuplicateEvents">Remove dupicate events (with the same UUID)</a> (Hotfix 2.3.115: In some rare situations it could occur that a duplicate of an event was created on an instance, with the exact same uuid. This action will remove any such duplicates and make sure that this cannot happen again.)</li>
<li><a href="/attributes/pruneOrphanedAttributes">Prune orphaned attributes</a> (In some rare occasions it can happen that you end up with some attributes in your database that do not belong to an event - for example during a race condition between an event insert and a delete. This tool will collect and delete any such orphaned attributes. If you ever run into an issue where you cannot add an attribute with a specific valid value, this is probably the reason.)</li>
<li><?php echo $this->Form->postLink('Extend Organization length', '/servers/updateDatabase/extendServerOrganizationLength');?> (Hotfix 2.3.57: Increase the max length of the organization field when adding a new server connection.)</li>
<li><?php echo $this->Form->postLink('Convert log fields to text', '/servers/updateDatabase/convertLogFieldsToText');?> (Hotfix 2.3.78: Some of the log fields that were varchar(255) ended up truncating the data. This function will change them to "text")</li>
<li><?php echo $this->Form->postLink('Fix duplicate UUIDs', '/servers/pruneDuplicateUUIDs');?> (Hotfix 2.3.107: it was previously possible to get duplicate attribute UUIDs in the database, this script will remove all duplicates and ensure that duplicates will not be entered into the database in the future.)</li>
<li><?php echo $this->Form->postLink('Remove dupicate events (with the same UUID)', '/servers/removeDuplicateEvents');?> (Hotfix 2.3.115: In some rare situations it could occur that a duplicate of an event was created on an instance, with the exact same uuid. This action will remove any such duplicates and make sure that this cannot happen again.)</li>
<li><?php echo $this->Form->postLink('Prune orphaned attributes', '/attributes/pruneOrphanedAttributes');?> (In some rare occasions it can happen that you end up with some attributes in your database that do not belong to an event - for example during a race condition between an event insert and a delete. This tool will collect and delete any such orphaned attributes. If you ever run into an issue where you cannot add an attribute with a specific valid value, this is probably the reason.)</li>
</ul>
</div>
<?php