Skip to content

Commit

Permalink
Merge pull request #1119 from Admidio/improve-csrf-token-support
Browse files Browse the repository at this point in the history
Improve csrf token support
  • Loading branch information
Fasse committed Oct 20, 2021
2 parents 0cf2055 + 87f304f commit eec4530
Show file tree
Hide file tree
Showing 38 changed files with 442 additions and 334 deletions.
2 changes: 2 additions & 0 deletions adm_program/installation/db_scripts/db.sql
Expand Up @@ -348,6 +348,7 @@ CREATE TABLE %PREFIX%_members
mem_id integer unsigned NOT NULL AUTO_INCREMENT,
mem_rol_id integer unsigned NOT NULL,
mem_usr_id integer unsigned NOT NULL,
mem_uuid varchar(36) NOT NULL,
mem_begin date NOT NULL,
mem_end date NOT NULL DEFAULT '9999-12-31',
mem_leader boolean NOT NULL DEFAULT '0',
Expand All @@ -365,6 +366,7 @@ DEFAULT character SET = utf8
COLLATE = utf8_unicode_ci;

CREATE INDEX %PREFIX%_idx_mem_rol_usr_id ON %PREFIX%_members (mem_rol_id, mem_usr_id);
CREATE UNIQUE INDEX %PREFIX%_idx_mem_uuid ON %PREFIX%_members (mem_uuid);

/*==============================================================*/
/* Table: adm_menu */
Expand Down
3 changes: 3 additions & 0 deletions adm_program/installation/db_scripts/update_4_1.xml
Expand Up @@ -70,6 +70,7 @@
<step id="335">ALTER TABLE %PREFIX%_rooms ADD COLUMN room_uuid varchar(36)</step>
<step id="340">ALTER TABLE %PREFIX%_user_fields ADD COLUMN usf_uuid varchar(36)</step>
<step id="350">ALTER TABLE %PREFIX%_user_relation_types ADD COLUMN urt_uuid varchar(36)</step>
<step id="360">ALTER TABLE %PREFIX%_members ADD COLUMN mem_uuid varchar(36)</step>
<step id="400">ComponentUpdateSteps::updateStep41AddUuid</step>
<step id="410" database="mysql">ALTER TABLE %PREFIX%_users MODIFY COLUMN usr_uuid varchar(36) NOT NULL</step>
<step id="420">CREATE UNIQUE INDEX %PREFIX%_idx_usr_uuid ON %PREFIX%_users (usr_uuid)</step>
Expand Down Expand Up @@ -109,5 +110,7 @@
<step id="760">CREATE UNIQUE INDEX %PREFIX%_idx_urt_uuid ON %PREFIX%_user_relation_types (urt_uuid)</step>
<step id="770" database="mysql">ALTER TABLE %PREFIX%_roles MODIFY COLUMN rol_name varchar(100)</step>
<step id="780" database="pgsql">ALTER TABLE %PREFIX%_roles ALTER COLUMN rol_name TYPE varchar(100)</step>
<step id="790" database="mysql">ALTER TABLE %PREFIX%_members MODIFY COLUMN mem_uuid varchar(36) NOT NULL</step>
<step id="800">CREATE UNIQUE INDEX %PREFIX%_idx_mem_uuid ON %PREFIX%_members (mem_uuid)</step>
<step>stop</step>
</update>
19 changes: 16 additions & 3 deletions adm_program/modules/announcements/announcements_function.php
Expand Up @@ -55,6 +55,22 @@

$_SESSION['announcements_request'] = $_POST;

try
{
// check the CSRF token of the form against the session token
SecurityUtils::validateCsrfToken($_POST['admidio-csrf-token']);
}
catch(AdmException $e)
{
if($getMode === 1) {
$e->showHtml();
}
else {
$e->showText();
}
// => EXIT
}

if($getMode === 1)
{
if(strlen($_POST['ann_headline']) === 0)
Expand All @@ -79,9 +95,6 @@

try
{
// check the CSRF token of the form against the session token
SecurityUtils::validateCsrfToken($_POST['admidio-csrf-token']);

// write POST parameters in announcement object
foreach($_POST as $key => $value) // TODO possible security issue
{
Expand Down
29 changes: 19 additions & 10 deletions adm_program/modules/backup/backup_file_function.php
@@ -1,17 +1,17 @@
<?php
/**
***********************************************************************************************
* Backup
* Backup functions
*
* @copyright 2004-2021 The Admidio Team
* @see https://www.admidio.org/
* @license https://www.gnu.org/licenses/gpl-2.0.html GNU General Public License v2.0 only
*
* Parameters:
*
* job - get_file : die uebergebene Backupdatei wird heruntergeladen
* - delete : die uebergebene Backupdatei wird geloescht
* filename : Der Name der Datei, welche heruntergeladen werden soll
* job - get_file : download backup file
* - delete : delete backup file
* filename : The name of the file to be used
***********************************************************************************************
*/
require_once(__DIR__ . '/../../system/common.php');
Expand All @@ -30,10 +30,10 @@

$backupAbsolutePath = ADMIDIO_PATH . FOLDER_DATA . '/backup/'; // make sure to include trailing slash

// kompletten Pfad der Datei holen
// get complete path of the file
$completePath = $backupAbsolutePath.$getFilename;

// pruefen ob File ueberhaupt physikalisch existiert
// check if file exists physically at all
if(!is_file($completePath))
{
$gMessage->show($gL10n->get('SYS_FILE_NOT_EXIST'));
Expand All @@ -43,11 +43,11 @@
switch($getJob)
{
case 'get_file':
// Dateigroese ermitteln
// Determine file size
$fileSize = filesize($completePath);
$filename = FileSystemUtils::getSanitizedPathEntry($getFilename);

// Passenden Datentyp erzeugen.
// Create suitable data type
header('Content-Type: application/octet-stream');
header('Content-Length: '.$fileSize);
header('Content-Disposition: attachment; filename="'.$filename.'"');
Expand All @@ -56,17 +56,26 @@
header('Cache-Control: private');
header('Pragma: public');

// Datei ausgeben.
// get file output
readfile($completePath);
break;

case 'delete':
// Backupdatei loeschen
// Delete backup file

try
{
// check the CSRF token of the form against the session token
SecurityUtils::validateCsrfToken($_POST['admidio-csrf-token']);

FileSystemUtils::deleteFileIfExists($completePath);
echo 'done';
}
catch(AdmException $e)
{
$e->showText();
// => EXIT
}
catch (\RuntimeException $exception)
{
$gLogger->error('Could not delete file!', array('filePath' => $completePath));
Expand Down
6 changes: 4 additions & 2 deletions adm_program/modules/categories/categories.php
Expand Up @@ -194,10 +194,12 @@
if($category->getValue('cat_system') == 0 || $getType !== 'USF')
{
$htmlMoveRow = '<a class="admidio-icon-link" href="javascript:void(0)" onclick="moveTableRow(\''.TableCategory::MOVE_UP.'\', \'row_'.$categoryUuid.'\',
\''.SecurityUtils::encodeUrl(ADMIDIO_URL . FOLDER_MODULES . '/categories/categories_function.php', array('type' => $getType, 'mode' => 4, 'cat_uuid' => $categoryUuid, 'sequence' => TableCategory::MOVE_UP)) . '\')">'.
\''.SecurityUtils::encodeUrl(ADMIDIO_URL . FOLDER_MODULES . '/categories/categories_function.php', array('type' => $getType, 'mode' => 4, 'cat_uuid' => $categoryUuid, 'sequence' => TableCategory::MOVE_UP)) . '\',
\''.$gCurrentSession->getCsrfToken().'\')">'.
'<i class="fas fa-chevron-circle-up" data-toggle="tooltip" title="' . $gL10n->get('SYS_MOVE_UP', array($addButtonText)) . '"></i></a>
<a class="admidio-icon-link" href="javascript:void(0)" onclick="moveTableRow(\''.TableCategory::MOVE_DOWN.'\', \'row_'.$categoryUuid.'\',
\''.SecurityUtils::encodeUrl(ADMIDIO_URL . FOLDER_MODULES . '/categories/categories_function.php', array('type' => $getType, 'mode' => 4, 'cat_uuid' => $categoryUuid, 'sequence' => TableCategory::MOVE_DOWN)) . '\')">'.
\''.SecurityUtils::encodeUrl(ADMIDIO_URL . FOLDER_MODULES . '/categories/categories_function.php', array('type' => $getType, 'mode' => 4, 'cat_uuid' => $categoryUuid, 'sequence' => TableCategory::MOVE_DOWN)) . '\',
\''.$gCurrentSession->getCsrfToken().'\')">'.
'<i class="fas fa-chevron-circle-down" data-toggle="tooltip" title="' . $gL10n->get('SYS_MOVE_DOWN', array($addButtonText)) . '"></i></a>';
}

Expand Down
36 changes: 23 additions & 13 deletions adm_program/modules/categories/categories_function.php
Expand Up @@ -28,7 +28,6 @@
$getCatUuid = admFuncVariableIsValid($_GET, 'cat_uuid', 'string');
$getType = admFuncVariableIsValid($_GET, 'type', 'string', array('requireValue' => true, 'validValues' => array('ROL', 'LNK', 'USF', 'ANN', 'DAT', 'AWA')));
$getMode = admFuncVariableIsValid($_GET, 'mode', 'int', array('requireValue' => true));
$getTitle = admFuncVariableIsValid($_GET, 'title', 'string', array('defaultValue' => $gL10n->get('SYS_CATEGORY')));

// set text strings for the different modules
switch ($getType)
Expand Down Expand Up @@ -58,6 +57,22 @@
$component = '';
}

try
{
// check the CSRF token of the form against the session token
SecurityUtils::validateCsrfToken($_POST['admidio-csrf-token']);
}
catch(AdmException $exception)
{
if($getMode === 1) {
$exception->showHtml();
}
else {
$exception->showText();
}
// => EXIT
}

// check if the current user has the right to
if(!Component::isAdministrable($component))
{
Expand Down Expand Up @@ -99,17 +114,6 @@

$_SESSION['categories_request'] = $_POST;

try
{
// check the CSRF token of the form against the session token
SecurityUtils::validateCsrfToken($_POST['admidio-csrf-token']);
}
catch(AdmException $exception)
{
$exception->showHtml();
// => EXIT
}

if((!array_key_exists('cat_name', $_POST) || $_POST['cat_name'] === '') && $category->getValue('cat_system') == 0)
{
$gMessage->show($gL10n->get('SYS_FIELD_EMPTY', array($gL10n->get('SYS_NAME'))));
Expand Down Expand Up @@ -268,6 +272,7 @@
if($category->delete())
{
echo 'done';
exit();
}
}
catch(AdmException $e)
Expand All @@ -281,6 +286,11 @@
// Kategoriereihenfolge aktualisieren
$getSequence = admFuncVariableIsValid($_GET, 'sequence', 'string', array('validValues' => array(TableCategory::MOVE_UP, TableCategory::MOVE_DOWN)));

$category->moveSequence($getSequence);
if($category->moveSequence($getSequence)) {
echo 'done';
}
else {
echo 'Sequence could not be changed.';
}
exit();
}
19 changes: 14 additions & 5 deletions adm_program/modules/dates/dates_function.php
Expand Up @@ -10,11 +10,10 @@
* Parameters:
*
* dat_id - ID of the event that should be edited
* mode : 1 - Create a new event
* mode : 1 - Create or edit an event
* 2 - Delete the event
* 3 - User attends to the event
* 4 - User cancel the event
* 5 - Edit an existing event
* 6 - Export event in ical format
* 7 - User may participate in the event
* user_uuid : UUID of the user membership to an event should be edited
Expand Down Expand Up @@ -70,7 +69,7 @@
$user->readDataByUuid($getUserUuid);
}

if (in_array($getMode, array(1, 2, 5), true))
if (in_array($getMode, array(1, 2), true))
{
if ($getDateUuid !== '')
{
Expand All @@ -92,9 +91,10 @@
}
}

if($getMode === 1 || $getMode === 5) // Create a new event or edit an existing event
if($getMode === 1) // Create a new event or edit an existing event
{
$_SESSION['dates_request'] = $_POST;
$dateIsNew = $date->isNewRecord();

try
{
Expand Down Expand Up @@ -446,7 +446,7 @@
{
$notification = new Email();

if($getMode === 1)
if($dateIsNew)
{
$message = $gL10n->get('DAT_EMAIL_NOTIFICATION_MESSAGE_PART1', array($gCurrentOrganization->getValue('org_longname'), $_POST['dat_headline'], $date->getDateTimePeriod(), $calendar))
.$gL10n->get('DAT_EMAIL_NOTIFICATION_MESSAGE_PART2', array($ort, $raum, $participants, $gCurrentUser->getValue('FIRST_NAME').' '.$gCurrentUser->getValue('LAST_NAME')))
Expand Down Expand Up @@ -583,6 +583,15 @@
}
elseif($getMode === 2)
{
try {
// check the CSRF token of the form against the session token
SecurityUtils::validateCsrfToken($_POST['admidio-csrf-token']);
}
catch(AdmException $exception) {
$exception->showText();
// => EXIT
}

// delete current announcements, right checks were done before
$date->delete();

Expand Down
5 changes: 1 addition & 4 deletions adm_program/modules/dates/dates_new.php
Expand Up @@ -40,17 +40,14 @@
if($getCopy)
{
$headline = $gL10n->get('SYS_COPY_VAR', array($getHeadline));
$mode = 5;
}
elseif($getDateUuid !== '')
{
$headline = $gL10n->get('SYS_EDIT_VAR', array($getHeadline));
$mode = 5;
}
else
{
$headline = $gL10n->get('SYS_CREATE_VAR', array($getHeadline));
$mode = 1;
}

$gNavigation->addUrl(CURRENT_URL, $headline);
Expand Down Expand Up @@ -250,7 +247,7 @@ function setLocationCountry() {
);

// show form
$form = new HtmlForm('dates_edit_form', SecurityUtils::encodeUrl(ADMIDIO_URL.FOLDER_MODULES.'/dates/dates_function.php', array('dat_uuid' => $getDateUuid, 'mode' => $mode, 'copy' => $getCopy)), $page);
$form = new HtmlForm('dates_edit_form', SecurityUtils::encodeUrl(ADMIDIO_URL.FOLDER_MODULES.'/dates/dates_function.php', array('dat_uuid' => $getDateUuid, 'mode' => 1, 'copy' => $getCopy)), $page);

$form->openGroupBox('gb_title_location', $gL10n->get('SYS_TITLE').' & '.$gL10n->get('DAT_LOCATION'));
$form->addInput(
Expand Down
15 changes: 8 additions & 7 deletions adm_program/modules/documents-files/documents_files_function.php
Expand Up @@ -68,15 +68,16 @@
}

// check the CSRF token of the form against the session token
if(in_array($getMode, array(3, 4, 7)))
{
try
{
if(in_array($getMode, array(2, 3, 4, 5, 7))) {
try {
SecurityUtils::validateCsrfToken($_POST['admidio-csrf-token']);
}
catch(AdmException $exception)
{
$exception->showHtml();
catch(AdmException $exception) {
if($getMode === 2 || $getMode === 5) {
$exception->showText();
} else {
$exception->showHtml();
}
// => EXIT
}
}
Expand Down

0 comments on commit eec4530

Please sign in to comment.