Skip to content

Commit

Permalink
#7371 Add missing CSRF checks (main branch)
Browse files Browse the repository at this point in the history
  • Loading branch information
@asmecher
asmecher committed Oct 12, 2021
1 parent 1a94bff commit 62c07e1
Show file tree
Hide file tree
Showing 10 changed files with 101 additions and 18 deletions.
27 changes: 23 additions & 4 deletions classes/linkAction/request/AjaxAction.inc.php
View file
Expand Up @@ -14,6 +14,8 @@

namespace PKP\linkAction\request;

use APP\core\Application;

class AjaxAction extends LinkActionRequest
{
public const AJAX_REQUEST_TYPE_GET = 'get';
Expand All @@ -25,18 +27,24 @@ class AjaxAction extends LinkActionRequest
/** @var string */
public $_requestType;

/** @var array */
public $_requestData;

/**
* Constructor
*
* @param $remoteAction string The target URL.
* @param $requestType string One of the AJAX_REQUEST_TYPE_* constants.
* @param $requestData array Any request data (e.g. POST params) to be sent.
*/
public function __construct($remoteAction, $requestType = self::AJAX_REQUEST_TYPE_POST)
public function __construct($remoteAction, $requestType = self::AJAX_REQUEST_TYPE_POST, $requestData = [])
{
parent::__construct();
$this->_remoteAction = $remoteAction;
$this->_requestType = $requestType;
$this->_requestData = array_merge($requestData, [
'csrfToken' => Application::getRequest()->getSession()->getCSRFToken(),
]);
}


Expand All @@ -54,15 +62,25 @@ public function getRemoteAction()
}

/**
* Get the modal object.
* Get the request type.
*
* @return Modal
* @return string
*/
public function getRequestType()
{
return $this->_requestType;
}

/**
* Get the request data.
*
* @return array
*/
public function getRequestData()
{
return $this->_requestData;
}


//
// Overridden protected methods from LinkActionRequest
Expand All @@ -82,7 +100,8 @@ public function getLocalizedOptions()
{
return [
'url' => $this->getRemoteAction(),
'requestType' => $this->getRequestType()
'requestType' => $this->getRequestType(),
'data' => $this->getRequestData(),
];
}
}
Expand Down
8 changes: 7 additions & 1 deletion classes/plugins/ImportExportPlugin.inc.php
View file
Expand Up @@ -400,7 +400,13 @@ public function getBounceTab($request, $title, $bounceUrl, $bounceParameterArray
$json = new JSONMessage(true);
$json->setEvent('addTab', [
'title' => $title,
'url' => $request->url(null, null, null, ['plugin', $this->getName(), $bounceUrl], $bounceParameterArray),
'url' => $request->url(
null,
null,
null,
['plugin', $this->getName(), $bounceUrl],
array_merge($bounceParameterArray, ['csrfToken' => $request->getSession()->getCSRFToken()])
),
]);
header('Content-Type: application/json');
return $json->getString();
Expand Down
6 changes: 6 additions & 0 deletions controllers/grid/languages/LanguageGridHandler.inc.php
View file
Expand Up @@ -83,6 +83,9 @@ protected function getRowInstance()
*/
public function saveLanguageSetting($args, $request)
{
if (!$request->checkCSRF()) {
return new JSONMessage(false);
}
$locale = (string) $request->getUserVar('rowId');
$settingName = (string) $request->getUserVar('setting');
$settingValue = (bool) $request->getUserVar('value');
Expand Down Expand Up @@ -180,6 +183,9 @@ public function saveLanguageSetting($args, $request)
*/
public function setContextPrimaryLocale($args, $request)
{
if (!$request->checkCSRF()) {
return new JSONMessage(false);
}
$locale = (string) $request->getUserVar('rowId');
$context = $request->getContext();
$availableLocales = $this->getGridDataElements($request);
Expand Down
9 changes: 9 additions & 0 deletions controllers/grid/notifications/NotificationsGridHandler.inc.php
View file
Expand Up @@ -184,6 +184,9 @@ protected function getNotificationsColumnTitle()
*/
public function markNew($args, $request)
{
if (!$request->checkCSRF()) {
return new JSONMessage(false);
}
$notificationDao = DAORegistry::getDAO('NotificationDAO'); /** @var NotificationDAO $notificationDao */
$user = $request->getUser();

Expand All @@ -209,6 +212,9 @@ public function markNew($args, $request)
*/
public function markRead($args, $request)
{
if (!$request->checkCSRF()) {
return new JSONMessage(false);
}
$notificationDao = DAORegistry::getDAO('NotificationDAO'); /** @var NotificationDAO $notificationDao */
$user = $request->getUser();

Expand Down Expand Up @@ -242,6 +248,9 @@ public function markRead($args, $request)
*/
public function deleteNotifications($args, $request)
{
if (!$request->checkCSRF()) {
return new JSONMessage(false);
}
$notificationDao = DAORegistry::getDAO('NotificationDAO'); /** @var NotificationDAO $notificationDao */
$user = $request->getUser();

Expand Down
3 changes: 3 additions & 0 deletions controllers/grid/settings/roles/UserGroupGridHandler.inc.php
View file
Expand Up @@ -416,6 +416,9 @@ public function unassignStage($args, $request)
*/
private function _toggleAssignment($args, $request)
{
if (!$request->checkCSRF()) {
return new JSONMessage(false);
}
$userGroup = $this->_userGroup;
$stageId = $this->getAuthorizedContextObject(ASSOC_TYPE_WORKFLOW_STAGE);
$contextId = $this->_getContextId();
Expand Down
7 changes: 4 additions & 3 deletions js/classes/linkAction/AjaxRequest.js
View file
Expand Up @@ -21,7 +21,8 @@
* @param {jQueryObject} $linkActionElement The element the link
* action was attached to.
* @param {{
* requestType: string
* requestType: string,
* data: PlainObject
* }} options Configuration of the link action
* request.
*/
Expand Down Expand Up @@ -51,11 +52,11 @@
this.handleResponse, this);
switch (options.requestType) {
case 'get':
$.getJSON(options.url, responseHandler);
$.getJSON(options.url, options.data, responseHandler);
break;

case 'post':
$.post(options.url, responseHandler, 'json');
$.post(options.url, options.data, responseHandler, 'json');
break;
}
return returnValue;
Expand Down
23 changes: 18 additions & 5 deletions pages/admin/AdminHandler.inc.php
View file
Expand Up @@ -404,9 +404,13 @@ public function phpinfo()
*/
public function expireSessions($args, $request)
{
if (!$request->checkCSRF()) {
return new JSONMessage(false);
}

$sessionDao = DAORegistry::getDAO('SessionDAO'); /** @var SessionDAO $sessionDao */
$sessionDao->deleteAllSessions();
$request->redirect(null, 'admin');
$request->redirect(null, 'login');
}

/**
Expand All @@ -417,6 +421,10 @@ public function expireSessions($args, $request)
*/
public function clearTemplateCache($args, $request)
{
if (!$request->checkCSRF()) {
return new JSONMessage(false);
}

$templateMgr = TemplateManager::getManager($request);
$templateMgr->clearTemplateCache();
$templateMgr->clearCssCache();
Expand All @@ -431,6 +439,10 @@ public function clearTemplateCache($args, $request)
*/
public function clearDataCache($args, $request)
{
if (!$request->checkCSRF()) {
return new JSONMessage(false);
}

// Clear the CacheManager's caches
$cacheManager = CacheManager::getManager();
$cacheManager->flush();
Expand All @@ -441,10 +453,8 @@ public function clearDataCache($args, $request)
/**
* Download scheduled task execution log file.
*/
public function downloadScheduledTaskLogFile()
public function downloadScheduledTaskLogFile($args, $request)
{
$request = Application::get()->getRequest();

$file = basename($request->getUserVar('file'));
ScheduledTaskHelper::downloadExecutionLog($file);
}
Expand All @@ -454,9 +464,12 @@ public function downloadScheduledTaskLogFile()
*/
public function clearScheduledTaskLogFiles()
{
if (!$request->checkCSRF()) {
return new JSONMessage(false);
}

ScheduledTaskHelper::clearExecutionLogs();

$request = Application::get()->getRequest();
$request->redirect(null, 'admin');
}
}
3 changes: 3 additions & 0 deletions plugins/importexport/native/PKPNativeImportExportPlugin.inc.php
View file
Expand Up @@ -198,6 +198,9 @@ public function display($args, $request)

break;
case 'import':
if (!$request->checkCSRF()) {
throw new Exception('CSRF mismatch!');
}
$temporaryFilePath = $this->getImportedFilePath($request->getUserVar('temporaryFileId'), $user);
[$filter, $xmlString] = $this->getImportFilter($temporaryFilePath);
$result = $this->getImportTemplateResult($filter, $xmlString, $this->getDeployment(), $templateMgr);
Expand Down
5 changes: 4 additions & 1 deletion plugins/importexport/users/PKPUserImportExportPlugin.inc.php
View file
Expand Up @@ -116,11 +116,14 @@ public function display($args, $request)
$json = new JSONMessage(true);
$json->setEvent('addTab', [
'title' => __('plugins.importexport.users.results'),
'url' => $request->url(null, null, null, ['plugin', $this->getName(), 'import'], ['temporaryFileId' => $request->getUserVar('temporaryFileId')]),
'url' => $request->url(null, null, null, ['plugin', $this->getName(), 'import'], ['temporaryFileId' => $request->getUserVar('temporaryFileId'), 'csrfToken' => $request->getSession()->getCSRFToken()]),
]);
header('Content-Type: application/json');
return $json->getString();
case 'import':
if (!$request->checkCSRF()) {
throw new Exception('CSRF mismatch!');
}
$temporaryFileId = $request->getUserVar('temporaryFileId');
$temporaryFileDao = DAORegistry::getDAO('TemporaryFileDAO'); /** @var TemporaryFileDAO $temporaryFileDao */
$user = $request->getUser();
Expand Down
28 changes: 24 additions & 4 deletions templates/admin/index.tpl
View file
Expand Up @@ -31,10 +31,30 @@
<h2>{translate key="admin.adminFunctions"}</h2>
<ul>
<li><a href="{url op="systemInfo"}">{translate key="admin.systemInformation"}</a></li>
<li><a href="{url op="expireSessions"}" onclick="return confirm({translate|json_encode|escape key="admin.confirmExpireSessions"})">{translate key="admin.expireSessions"}</a></li>
<li><a href="{url op="clearDataCache"}">{translate key="admin.clearDataCache"}</a></li>
<li><a href="{url op="clearTemplateCache"}" onclick="return confirm({translate|json_encode|escape key="admin.confirmClearTemplateCache"})">{translate key="admin.clearTemplateCache"}</a></li>
<li><a href="{url op="clearScheduledTaskLogFiles"}" onclick="return confirm({translate|json_encode|escape key="admin.scheduledTask.confirmClearLogs"})">{translate key="admin.scheduledTask.clearLogs"}</a></li>
<li>
<form type="post" action="{url op="expireSessions"}">
{csrf}
<button class="-linkButton" onclick="return confirm({translate|json_encode|escape key="admin.confirmExpireSessions"})">{translate key="admin.expireSessions"}</button>
</form>
</li>
<li>
<form type="post" action="{url op="clearDataCache"}">
{csrf}
<button class="-linkButton">{translate key="admin.clearDataCache"}</button>
</form>
</li>
<li>
<form type="post" action="{url op="clearTemplateCache"}">
{csrf}
<button class="-linkButton" onclick="return confirm({translate|json_encode|escape key="admin.confirmClearTemplateCache"})">{translate key="admin.clearTemplateCache"}</button>
</form>
</li>
<li>
<form type="post" action="{url op="clearScheduledTaskLogFiles"}">
{csrf}
<button class="-linkButton" onclick="return confirm({translate|json_encode|escape key="admin.scheduledTask.confirmClearLogs"})">{translate key="admin.scheduledTask.clearLogs"}</button>
</form>
</li>
{call_hook name="Templates::Admin::Index::AdminFunctions"}
</ul>
</div>
Expand Down

0 comments on commit 62c07e1

Please sign in to comment.