Skip to content

Commit

Permalink
fix: [security] Fix to a vulnerability related to the server index
Browse files Browse the repository at this point in the history 
- along with various support tools
- more information coming soon
  • Loading branch information
@iglocska
iglocska committed Sep 9, 2019
1 parent 36ef323 commit 75acd63
Show file tree
Hide file tree
Showing 9 changed files with 436 additions and 77 deletions.
26 changes: 26 additions & 0 deletions app/Console/Command/AdminShell.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -506,4 +506,30 @@ public function cleanCaches()
$this->Server->cleanCacheFiles();
echo '...caches lost in time, like tears in rain.' . PHP_EOL;
}

public function resetSyncAuthkeys()
{
if (empty($this->args[0])) {
echo sprintf(
__("MISP mass sync authkey reset command line tool.\n\nUsage: %sConsole/cake resetSyncAuthkeys [user_id]") . "\n\n",
APP
);
die();
} else {
$userId = $this->args[0];
$user = $this->User->getAuthUser($userId);
if (empty($user)) {
echo __('Invalid user.') . "\n\n";
}
if (!$user['Role']['perm_site_admin']) {
echo __('User has to be a site admin.') . "\n\n";
}
if (!empty($this->args[1])) {
$jobId = $this->args[1];
} else {
$jobId = false;
}
$this->User->resetAllSyncAuthKeys($user, $jobId);
}
}
}
88 changes: 81 additions & 7 deletions app/Controller/Component/ACLComponent.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -335,7 +335,7 @@ class ACLComponent extends Component
),
'servers' => array(
'add' => array(),
'cache' => array('perm_site_admin'),
'cache' => array(),
'checkout' => array(),
'createSync' => array('perm_sync'),
'delete' => array(),
Expand All @@ -348,14 +348,14 @@ class ACLComponent extends Component
'getInstanceUUID' => array('perm_sync'),
'getPyMISPVersion' => array('*'),
'getSetting' => array(),
'getSubmodulesStatus' => array('perm_site_admin'),
'getSubmoduleQuickUpdateForm' => array('perm_site_admin'),
'getSubmodulesStatus' => array(),
'getSubmoduleQuickUpdateForm' => array(),
'getWorkers' => array(),
'getVersion' => array('*'),
'import' => ('perm_site_admin'),
'index' => array('OR' => array('perm_sync', 'perm_admin')),
'import' => array(),
'index' => array(),
'ondemandAction' => array(),
'postTest' => array('perm_sync'),
'postTest' => array(),
'previewEvent' => array(),
'previewIndex' => array(),
'pull' => array(),
Expand All @@ -371,7 +371,7 @@ class ACLComponent extends Component
'statusZeroMQServer' => array(),
'stopWorker' => array(),
'stopZeroMQServer' => array(),
'testConnection' => array('perm_sync'),
'testConnection' => array(),
'update' => array(),
'updateJSON' => array(),
'updateProgress' => array(),
Expand Down Expand Up @@ -518,6 +518,7 @@ class ACLComponent extends Component
'initiatePasswordReset' => array('perm_admin'),
'login' => array('*'),
'logout' => array('*'),
'resetAllSyncAuthKeys' => array(),
'resetauthkey' => array('*'),
'request_API' => array('*'),
'routeafterlogin' => array('*'),
Expand Down Expand Up @@ -553,6 +554,78 @@ class ACLComponent extends Component
)
);

private function __checkLoggedActions($user, $controller, $action)
{
$loggedActions = array(
'servers' => array(
'index' => array(
'role' => array(
'NOT' => array(
'perm_site_admin'
)
),
'message' => __('This could be an indication of an attempted privilege escalation on older vulnerable versions of MISP (<2.4.115)')
)
)
);
foreach ($loggedActions as $k => $v) {
$loggedActions[$k] = array_change_key_case($v);
}
$message = '';
if (!empty($loggedActions[$controller])) {
if (!empty($loggedActions[$controller][$action])) {
$message = $loggedActions[$controller][$action]['message'];
$hit = false;
if (empty($loggedActions[$controller][$action]['role'])) {
$hit = true;
} else {
$role_req = $loggedActions[$controller][$action]['role'];
if (empty($role_req['OR']) && empty($role_req['AND']) && empty($role_req['NOT'])) {
$role_req = array('OR' => $role_req);
}
if (!empty($role_req['NOT'])) {
foreach ($role_req['NOT'] as $k => $v) {
if (!$user['Role'][$v]) {
$hit = true;
continue;
}
}
}
if (!$hit && !empty($role_req['AND'])) {
$subhit = true;
foreach ($role_req['AND'] as $k => $v) {
$subhit = $subhit && $user['Role'][$v];
}
if ($subhit) {
$hit = true;
}
}
if (!$hit && !empty($role_req['OR'])) {
foreach ($role_req['OR'] as $k => $v) {
if ($user['Role'][$v]) {
$hit = true;
continue;
}
}
}
if ($hit) {
$this->Log = ClassRegistry::init('Log');
$this->Log->create();
$this->Log->save(array(
'org' => 'SYSTEM',
'model' => 'User',
'model_id' => $user['id'],
'email' => $user['email'],
'action' => 'security',
'user_id' => $user['id'],
'title' => __('User triggered security alert by attempting to access /%s/%s. Reason why this endpoint is of interest: %s', $controller, $action, $message),
));
}
}
}
}
}

// The check works like this:
// If the user is a site admin, return true
// If the requested action has an OR-d list, iterate through the list. If any of the permissions are set for the user, return true
Expand All @@ -567,6 +640,7 @@ public function checkAccess($user, $controller, $action, $soft = false)
foreach ($aclList as $k => $v) {
$aclList[$k] = array_change_key_case($v);
}
$this->__checkLoggedActions($user, $controller, $action);
if ($user['Role']['perm_site_admin']) {
return true;
}
Expand Down
30 changes: 24 additions & 6 deletions app/Controller/ServersController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,12 +44,6 @@ public function beforeFilter()

public function index()
{
if (!$this->_isSiteAdmin()) {
if (!$this->userRole['perm_sync'] && !$this->userRole['perm_admin']) {
$this->redirect(array('controller' => 'events', 'action' => 'index'));
}
$this->paginate['conditions'] = array('Server.org_id LIKE' => $this->Auth->user('org_id'));
}
if ($this->_isRest()) {
$params = array(
'recursive' => -1,
Expand Down Expand Up @@ -2089,4 +2083,28 @@ public function import()
}
}
}

public function resetRemoteAuthKey($id)
{
if (!$this->request->is('post')) {
throw new MethodNotAllowedException(__('This endpoint expects POST requests.'));
}
$result = $this->Server->resetRemoteAuthkey($id);
if ($result !== true) {
if (!$this->_isRest()) {
$this->Flash->error($result);
$this->redirect(array('action' => 'index'));
} else {
return $this->RestResponse->saveFailResponse('Servers', 'resetRemoteAuthKey', $id, $message, $this->response->type());
}
} else {
$message = __('API key updated.');
if (!$this->_isRest()) {
$this->Flash->success($message);
$this->redirect(array('action' => 'index'));
} else {
return $this->RestResponse->saveSuccessResponse('Servers', 'resetRemoteAuthKey', $message, $this->response->type());
}
}
}
}
95 changes: 34 additions & 61 deletions app/Controller/UsersController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ public function change_pw()
// Save the data
if ($this->User->save($user)) {
$message = __('Password Changed.');
$this->__extralog("change_pw");
$this->User->extralog($this->Auth->user(), "change_pw", null, null, $user);
if ($this->_isRest()) {
return $this->RestResponse->saveSuccessResponse('User', 'change_pw', false, $this->response->type(), $message);
}
Expand Down Expand Up @@ -869,7 +869,11 @@ public function admin_edit($id = null)
$c++;
}
$fieldsResultStr = substr($fieldsResultStr, 2);
$this->__extralog("edit", "user", $fieldsResultStr);
$user = $this->User->find('first', array(
'recursive' => -1,
'conditions' => array('User.id' => $this->id)
));
$this->User->extralog($this->Auth->user(), "edit", "user", $fieldsResultStr, $user);
if ($this->_isRest()) {
$user = $this->User->find('first', array(
'conditions' => array('User.id' => $this->User->id),
Expand Down Expand Up @@ -954,7 +958,7 @@ public function admin_delete($id = null)
}
$fieldsDescrStr = 'User (' . $id . '): ' . $user['User']['email'];
if ($this->User->delete($id)) {
$this->__extralog("delete", $fieldsDescrStr, '');
$this->User->extralog($this->Auth->user(), "delete", $fieldsDescrStr, '');
if ($this->_isRest()) {
return $this->RestResponse->saveSuccessResponse('User', 'admin_delete', $id, $this->response->type(), 'User deleted.');
} else {
Expand Down Expand Up @@ -1010,7 +1014,7 @@ public function login()
}
}
if ($this->Auth->login()) {
$this->__extralog("login");
$this->User->extralog($this->Auth->user(), "login");
$this->User->Behaviors->disable('SysLogLogable.SysLogLogable');
$this->User->id = $this->Auth->user('id');
$user = $this->User->find('first', array(
Expand Down Expand Up @@ -1125,7 +1129,7 @@ public function routeafterlogin()
public function logout()
{
if ($this->Session->check('Auth.User')) {
$this->__extralog("logout");
$this->User->extralog($this->Auth->user(), "logout");
}
$this->Flash->info(__('Good-Bye'));
$user = $this->User->find('first', array(
Expand All @@ -1140,7 +1144,7 @@ public function logout()
$this->redirect($this->Auth->logout());
}

public function resetauthkey($id = null)
public function resetauthkey($id = null, $alert = false)
{
if (!$this->_isAdmin() && Configure::read('MISP.disableUserSelfManagement')) {
throw new MethodNotAllowedException('User self-management has been disabled on this instance.');
Expand All @@ -1149,24 +1153,12 @@ public function resetauthkey($id = null)
$id = $this->Auth->user('id');
}
if (!$this->userRole['perm_auth']) {
throw new MethodNotAllowedException('Invalid action.');
throw new MethodNotAllowedException(__('Invalid action.'));
}
$newkey = $this->User->resetauthkey($this->Auth->user(), $id, $alert);
if ($newkey === false) {
throw new MethodNotAllowedException(__('Invalid user.'));
}
$this->User->id = $id;
if (!$id || !$this->User->exists($id)) {
throw new MethodNotAllowedException('Invalid user.');
}
$user = $this->User->read();
$oldKey = $this->User->data['User']['authkey'];
if (!$this->_isSiteAdmin() && !($this->_isAdmin() && $this->Auth->user('org_id') == $this->User->data['User']['org_id']) && ($this->Auth->user('id') != $id)) {
throw new MethodNotAllowedException('Invalid user.');
}
$newkey = $this->User->generateAuthKey();
$this->User->saveField('authkey', $newkey);
$this->__extralog(
'reset_auth_key',
'Authentication key for user ' . $user['User']['id'] . ' (' . $user['User']['email'] . ')',
$fieldsResult = 'authkey(' . $oldKey . ') => (' . $newkey . ')'
);
if (!$this->_isRest()) {
$this->Flash->success(__('New authkey generated.', true));
$this->_refreshAuth();
Expand All @@ -1176,6 +1168,25 @@ public function resetauthkey($id = null)
}
}

public function resetAllSyncAuthKeys()
{
if (!$this->request->is('post')) {
throw new MethodNotAllowedException(__('This functionality is only accessible via POST requests.'));
}
$results = $this->User->resetAllSyncAuthKeysRouter($this->Auth->user());
if ($results === true) {
$message = __('Job initiated.');
} else {
$message = __('%s authkeys reset, %s could not be reset.', $results['success'], $results['fails']);
}
if (!$this->_isRest()) {
$this->Flash->info($message);
$this->redirect($this->referer());
} else {
return $this->RestResponse->saveSuccessResponse('User', 'resetAllSyncAuthKeys', false, $this->response->type(), $message);
}
}

public function histogram($selected = null)
{
//if (!$this->request->is('ajax') && !$this->_isRest()) throw new MethodNotAllowedException('This function can only be accessed via AJAX or the API.');
Expand Down Expand Up @@ -1297,44 +1308,6 @@ public function downloadTerms()
return $this->response;
}

private function __extralog($action = null, $description = null, $fieldsResult = null)
{
// new data
$model = 'User';
$modelId = $this->Auth->user('id');
if ($action == 'login') {
$description = "User (" . $this->Auth->user('id') . "): " . $this->data['User']['email'];
} elseif ($action == 'logout') {
$description = "User (" . $this->Auth->user('id') . "): " . $this->Auth->user('email');
} elseif ($action == 'edit') {
$description = "User (" . $this->User->id . "): " . $this->data['User']['email'];
} elseif ($action == 'change_pw') {
$description = "User (" . $this->User->id . "): " . $this->Auth->user('email');
$fieldsResult = "Password changed.";
}

// query
$this->Log = ClassRegistry::init('Log');
$this->Log->create();
$this->Log->save(array(
'org' => $this->Auth->user('Organisation')['name'],
'model' => $model,
'model_id' => $modelId,
'email' => $this->Auth->user('email'),
'action' => $action,
'title' => $description,
'change' => isset($fieldsResult) ? $fieldsResult : ''));

// write to syslogd as well
App::import('Lib', 'SysLog.SysLog');
$syslog = new SysLog();
if (isset($fieldsResult) && $fieldsResult) {
$syslog->write('notice', $description . ' -- ' . $action . ' -- ' . $fieldsResult);
} else {
$syslog->write('notice', $description . ' -- ' . $action);
}
}

// Used for fields_before and fields for audit
public function arrayCopy(array $array)
{
Expand Down
1 change: 1 addition & 0 deletions app/Model/Log.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ class Log extends AppModel
'request',
'request_delegation',
'reset_auth_key',
'security',
'serverSettingsEdit',
'tag',
'undelete',
Expand Down

0 comments on commit 75acd63

Please sign in to comment.