Add missing CSRF checks

This commit is contained in:
Frédéric Guillot 2021-06-05 14:33:19 -07:00 committed by fguillot
parent 41102ec161
commit 71123b0f37
23 changed files with 58 additions and 17 deletions

View File

@ -9,3 +9,5 @@ Only the latest stable version is supported.
Do not open a new GitHub issue if the bug is a security vulnerability.
Send an email to `security AT kanboard DOT net` with all the steps to reproduce the problem.
**This software is in maintenance mode**. Low severity or harmless issues won't be fixed.

View File

@ -33,6 +33,13 @@ abstract class BaseController extends Base
}
}
protected function checkReusableGETCSRFParam()
{
if (! $this->token->validateReusableCSRFToken($this->request->getStringParam('csrf_token'))) {
throw new AccessForbiddenException();
}
}
protected function checkCSRFForm()
{
if (! $this->token->validateCSRFToken($this->request->getRawValue('csrf_token'))) {

View File

@ -21,6 +21,7 @@ class BoardAjaxController extends BaseController
*/
public function save()
{
$this->checkReusableGETCSRFParam();
$project_id = $this->request->getIntegerParam('project_id');
if (! $project_id || ! $this->request->isAjax()) {

View File

@ -150,6 +150,7 @@ class ColumnController extends BaseController
*/
public function move()
{
$this->checkReusableGETCSRFParam();
$project = $this->getProject();
$values = $this->request->getJson();

View File

@ -166,6 +166,7 @@ class CommentController extends BaseController
*/
public function toggleSorting()
{
$this->checkReusableGETCSRFParam();
$task = $this->getTask();
$this->helper->comment->toggleSorting();

View File

@ -149,8 +149,12 @@ class ProjectViewController extends BaseController
*/
public function doDuplication()
{
$this->checkCSRFForm();
$project = $this->getProject();
$project_id = $this->projectDuplicationModel->duplicate($project['id'], array_keys($this->request->getValues()), $this->userSession->getId());
$values = $this->request->getRawFormValues();
$project_id = $this->projectDuplicationModel->duplicate($project['id'], array_keys($values), $this->userSession->getId());
if ($project_id !== false) {
$this->flash->success(t('Project cloned successfully.'));

View File

@ -17,6 +17,7 @@ class SubtaskStatusController extends BaseController
*/
public function change()
{
$this->checkReusableGETCSRFParam();
$task = $this->getTask();
$subtask = $this->getSubtask($task);
$fragment = $this->request->getStringParam('fragment');

View File

@ -205,6 +205,7 @@ class SwimlaneController extends BaseController
*/
public function move()
{
$this->checkReusableGETCSRFParam();
$project = $this->getProject();
$values = $this->request->getJson();

View File

@ -23,6 +23,13 @@ class TaskListController extends BaseController
$project = $this->getProject();
$search = $this->helper->projectHeader->getSearchQuery($project);
if ($this->request->getIntegerParam('show_subtasks') !== 0 ||
$this->request->getIntegerParam('hide_subtasks') !== 0 ||
$this->request->getStringParam('direction') !== '' ||
$this->request->getStringParam('order') !== '') {
$this->checkReusableGETCSRFParam();
}
if ($this->request->getIntegerParam('show_subtasks')) {
session_set('subtaskListToggle', true);
} elseif ($this->request->getIntegerParam('hide_subtasks')) {
@ -41,7 +48,7 @@ class TaskListController extends BaseController
$this->userSession->setListOrder($project['id'], $order, $direction);
$paginator = $this->paginator
->setUrl('TaskListController', 'show', array('project_id' => $project['id']))
->setUrl('TaskListController', 'show', array('project_id' => $project['id'], 'csrf_token' => $this->token->getReusableCSRFToken()))
->setMax(30)
->setOrder($order)
->setDirection($direction)

View File

@ -16,6 +16,7 @@ class TaskModificationController extends BaseController
{
public function assignToMe()
{
$this->checkReusableGETCSRFParam();
$task = $this->getTask();
$values = ['id' => $task['id'], 'owner_id' => $this->userSession->getId()];
@ -38,6 +39,7 @@ class TaskModificationController extends BaseController
*/
public function start()
{
$this->checkReusableGETCSRFParam();
$task = $this->getTask();
$values = ['id' => $task['id'], 'date_started' => time()];

View File

@ -31,6 +31,7 @@ class TaskMovePositionController extends BaseController
public function save()
{
$this->checkReusableGETCSRFParam();
$task = $this->getTask();
$values = $this->request->getJson();

View File

@ -34,6 +34,7 @@ class WebNotificationController extends BaseController
*/
public function flush()
{
$this->checkReusableGETCSRFParam();
$userId = $this->getUserId();
$this->userUnreadNotificationModel->markAllAsRead($userId);
$this->show();
@ -46,6 +47,7 @@ class WebNotificationController extends BaseController
*/
public function remove()
{
$this->checkReusableGETCSRFParam();
$user_id = $this->getUserId();
$notification_id = $this->request->getIntegerParam('notification_id');
$this->userUnreadNotificationModel->markAsRead($user_id, $notification_id);

View File

@ -111,6 +111,16 @@ class Request extends Base
return array();
}
/**
* Get POST values without modification
*
* @return array
*/
public function getRawFormValues()
{
return $this->post;
}
/**
* Get POST value without modification
*

View File

@ -64,6 +64,7 @@ class SubtaskHelper extends Base
'subtask_id' => $subtask['id'],
'user_id' => $userId,
'fragment' => $fragment,
'csrf_token' => $this->token->getReusableCSRFToken(),
);
if ($subtask['status'] == 0 && $this->hasSubtaskInProgress()) {

View File

@ -11,9 +11,9 @@
class="board-project-<?= $project['id'] ?>"
data-project-id="<?= $project['id'] ?>"
data-check-interval="<?= $board_private_refresh_interval ?>"
data-save-url="<?= $this->url->href('BoardAjaxController', 'save', array('project_id' => $project['id'])) ?>"
data-reload-url="<?= $this->url->href('BoardAjaxController', 'reload', array('project_id' => $project['id'])) ?>"
data-check-url="<?= $this->url->href('BoardAjaxController', 'check', array('project_id' => $project['id'], 'timestamp' => time())) ?>"
data-save-url="<?= $this->url->href('BoardAjaxController', 'save', array('project_id' => $project['id'], 'csrf_token' => $this->app->getToken()->getReusableCSRFToken())) ?>"
data-reload-url="<?= $this->url->href('BoardAjaxController', 'reload', array('project_id' => $project['id'], 'csrf_token' => $this->app->getToken()->getReusableCSRFToken())) ?>"
data-check-url="<?= $this->url->href('BoardAjaxController', 'check', array('project_id' => $project['id'], 'timestamp' => time(), 'csrf_token' => $this->app->getToken()->getReusableCSRFToken())) ?>"
data-task-creation-url="<?= $this->url->href('TaskCreationController', 'show', array('project_id' => $project['id'])) ?>"
>
<?php endif ?>

View File

@ -12,7 +12,7 @@
<?php else: ?>
<table
class="columns-table table-striped"
data-save-position-url="<?= $this->url->href('ColumnController', 'move', array('project_id' => $project['id'])) ?>">
data-save-position-url="<?= $this->url->href('ColumnController', 'move', array('project_id' => $project['id'], 'csrf_token' => $this->app->getToken()->getReusableCSRFToken())) ?>">
<thead>
<tr>
<th><?= t('Column') ?></th>

View File

@ -1,6 +1,6 @@
<table
class="swimlanes-table table-striped table-scrolling"
data-save-position-url="<?= $this->url->href('SwimlaneController', 'move', array('project_id' => $project['id'])) ?>">
data-save-position-url="<?= $this->url->href('SwimlaneController', 'move', array('project_id' => $project['id'], 'csrf_token' => $this->app->getToken()->getReusableCSRFToken())) ?>">
<thead>
<tr>
<th><?= t('Name') ?></th>

View File

@ -86,7 +86,7 @@
<?php endif ?>
</span>
<?php if ($editable && $task['owner_id'] != $this->user->getId()): ?>
- <span><?= $this->url->link(t('Assign to me'), 'TaskModificationController', 'assignToMe', ['task_id' => $task['id'], 'project_id' => $task['project_id']]) ?></span>
- <span><?= $this->url->link(t('Assign to me'), 'TaskModificationController', 'assignToMe', ['task_id' => $task['id'], 'project_id' => $task['project_id'], 'csrf_token' => $this->app->getToken()->getReusableCSRFToken()]) ?></span>
<?php endif ?>
</li>
<?php if ($task['creator_username']): ?>
@ -124,7 +124,7 @@
<?php if ($task['date_started']): ?>
<span><?= $this->dt->datetime($task['date_started']) ?></span>
<?php elseif ($editable): ?>
<span><?= $this->url->link(t('Start now'), 'TaskModificationController', 'start', ['task_id' => $task['id'], 'project_id' => $task['project_id']]) ?></span>
<span><?= $this->url->link(t('Start now'), 'TaskModificationController', 'start', ['task_id' => $task['id'], 'project_id' => $task['project_id'], 'csrf_token' => $this->app->getToken()->getReusableCSRFToken()]) ?></span>
<?php endif ?>
</li>
<li>

View File

@ -6,12 +6,12 @@
<?php if ($this->projectRole->canUpdateTask($task)): ?>
<?php if ($this->projectRole->canChangeAssignee($task) && array_key_exists('owner_id', $task) && $task['owner_id'] != $this->user->getId()): ?>
<li>
<?= $this->url->icon('hand-o-right', t('Assign to me'), 'TaskModificationController', 'assignToMe', ['task_id' => $task['id'], 'project_id' => $task['project_id'], 'redirect' => isset($redirect) ? $redirect : '']) ?>
<?= $this->url->icon('hand-o-right', t('Assign to me'), 'TaskModificationController', 'assignToMe', ['task_id' => $task['id'], 'project_id' => $task['project_id'], 'csrf_token' => $this->app->getToken()->getReusableCSRFToken(), 'redirect' => isset($redirect) ? $redirect : '']) ?>
</li>
<?php endif ?>
<?php if (array_key_exists('date_started', $task) && empty($task['date_started'])): ?>
<li>
<?= $this->url->icon('play', t('Set the start date automatically'), 'TaskModificationController', 'start', ['task_id' => $task['id'], 'project_id' => $task['project_id'], 'redirect' => isset($redirect) ? $redirect : '']) ?>
<?= $this->url->icon('play', t('Set the start date automatically'), 'TaskModificationController', 'start', ['task_id' => $task['id'], 'project_id' => $task['project_id'], 'csrf_token' => $this->app->getToken()->getReusableCSRFToken(), 'redirect' => isset($redirect) ? $redirect : '']) ?>
</li>
<?php endif ?>
<li>

View File

@ -4,7 +4,7 @@
<?php if (!isset($is_public) || !$is_public): ?>
<div class="comment-sorting">
<small>
<?= $this->url->icon('sort', t('Change sorting'), 'CommentController', 'toggleSorting', array('task_id' => $task['id'], 'project_id' => $task['project_id'])) ?>
<?= $this->url->icon('sort', t('Change sorting'), 'CommentController', 'toggleSorting', array('task_id' => $task['id'], 'project_id' => $task['project_id'], 'csrf_token' => $this->app->getToken()->getReusableCSRFToken())) ?>
<?php if ($editable): ?>
<?= $this->modal->medium('paper-plane', t('Send by email'), 'CommentMailController', 'create', array('task_id' => $task['id'], 'project_id' => $task['project_id'])) ?>
<?php endif ?>

View File

@ -30,9 +30,9 @@
<div class="table-list-header-menu">
<?php if (isset($project)): ?>
<?php if ($this->user->hasSubtaskListActivated()): ?>
<?= $this->url->icon('tasks', t('Hide subtasks'), 'TaskListController', 'show', array('project_id' => $project['id'], 'hide_subtasks' => 1)) ?>
<?= $this->url->icon('tasks', t('Hide subtasks'), 'TaskListController', 'show', array('project_id' => $project['id'], 'hide_subtasks' => 1, 'csrf_token' => $this->app->getToken()->getReusableCSRFToken())) ?>
<?php else: ?>
<?= $this->url->icon('tasks', t('Show subtasks'), 'TaskListController', 'show', array('project_id' => $project['id'], 'show_subtasks' => 1)) ?>
<?= $this->url->icon('tasks', t('Show subtasks'), 'TaskListController', 'show', array('project_id' => $project['id'], 'show_subtasks' => 1, 'csrf_token' => $this->app->getToken()->getReusableCSRFToken())) ?>
<?php endif ?>
<?php endif ?>

View File

@ -5,7 +5,7 @@
<form>
<?= $this->app->component('task-move-position', array(
'saveUrl' => $this->url->href('TaskMovePositionController', 'save', array('task_id' => $task['id'], 'project_id' => $task['project_id'])),
'saveUrl' => $this->url->href('TaskMovePositionController', 'save', array('task_id' => $task['id'], 'project_id' => $task['project_id'], 'csrf_token' => $this->app->getToken()->getReusableCSRFToken())),
'board' => $board,
'task' => $task,
'swimlaneLabel' => t('Swimlane'),

View File

@ -4,7 +4,7 @@
<?php if (! empty($notifications)): ?>
<ul>
<li>
<?= $this->modal->replaceIconLink('check-square-o', t('Mark all as read'), 'WebNotificationController', 'flush', array('user_id' => $user['id'])) ?>
<?= $this->modal->replaceIconLink('check-square-o', t('Mark all as read'), 'WebNotificationController', 'flush', array('user_id' => $user['id'], 'csrf_token' => $this->app->getToken()->getReusableCSRFToken())) ?>
</li>
</ul>
<?php endif ?>
@ -60,7 +60,7 @@
</span>
<div class="table-list-details">
<?= $this->dt->datetime($notification['date_creation']) ?>
<?= $this->modal->replaceIconLink('check', t('Mark as read'), 'WebNotificationController', 'remove', array('user_id' => $user['id'], 'notification_id' => $notification['id'])) ?>
<?= $this->modal->replaceIconLink('check', t('Mark as read'), 'WebNotificationController', 'remove', array('user_id' => $user['id'], 'notification_id' => $notification['id'], 'csrf_token' => $this->app->getToken()->getReusableCSRFToken())) ?>
</div>
</div>
<?php endforeach ?>