From c5bdbd16af8a4b1f2dec09a47c31cd5d2d240264 Mon Sep 17 00:00:00 2001 From: Jan-Hendrik Willms Date: Thu, 29 Aug 2024 14:38:15 +0000 Subject: use csrf protection correctly, fixes #4545 Closes #4545 Merge request studip/studip!3341 --- app/controllers/admin/login_style.php | 7 +- app/controllers/admin/user.php | 2 +- app/controllers/contact.php | 4 +- app/controllers/course/timesrooms.php | 6 +- app/controllers/course/wiki.php | 9 +- app/controllers/materialien/files.php | 31 +--- app/controllers/questionnaire.php | 5 +- app/controllers/settings/deputies.php | 2 +- app/controllers/shared/contacts.php | 4 +- app/controllers/studiengaenge/studiengaenge.php | 50 +++-- app/views/materialien/files/index.php | 237 ++++++++++++------------ app/views/materialien/files/range.php | 206 ++++++++++---------- app/views/shared/contacts/index.php | 171 ++++++++--------- lib/classes/forms/Form.php | 5 +- 14 files changed, 365 insertions(+), 374 deletions(-) diff --git a/app/controllers/admin/login_style.php b/app/controllers/admin/login_style.php index 35cd7d8..6f721aa 100644 --- a/app/controllers/admin/login_style.php +++ b/app/controllers/admin/login_style.php @@ -64,7 +64,8 @@ class Admin_LoginStyleController extends AuthenticatedController */ public function add_pic_action() { - CSRFProtection::verifyRequest(); + CSRFProtection::verifyUnsafeRequest(); + $success = 0; foreach ($_FILES['pictures']['name'] as $index => $filename) { if ($_FILES['pictures']['error'][$index] !== UPLOAD_ERR_OK) { @@ -174,7 +175,7 @@ class Admin_LoginStyleController extends AuthenticatedController public function store_faq_action(LoginFaq $entry = null) { - CSRFProtection::verifyRequest(); + CSRFProtection::verifyUnsafeRequest(); $entry->setData([ 'title' => Request::i18n('title'), @@ -190,7 +191,7 @@ class Admin_LoginStyleController extends AuthenticatedController public function delete_faq_action(LoginFaq $entry) { - CSRFProtection::verifyRequest(); + CSRFProtection::verifyUnsafeRequest(); if ($entry->delete()) { PageLayout::postSuccess(_('Der Hinweistext wurde gelöscht.')); diff --git a/app/controllers/admin/user.php b/app/controllers/admin/user.php index 8bc29db..d1dcf1e 100644 --- a/app/controllers/admin/user.php +++ b/app/controllers/admin/user.php @@ -1093,7 +1093,7 @@ class Admin_UserController extends AuthenticatedController */ public function store_user_institute_action($user_id, $institute_id) { - CSRFProtection::verifyRequest(); + CSRFProtection::verifyUnsafeRequest(); $inst_membership = InstituteMember::findOneBySQL('user_id = ? AND institut_id = ?', [$user_id, $institute_id]); diff --git a/app/controllers/contact.php b/app/controllers/contact.php index 2148777..7820ce6 100644 --- a/app/controllers/contact.php +++ b/app/controllers/contact.php @@ -166,7 +166,7 @@ class ContactController extends AuthenticatedController $this->group->owner_id = User::findCurrent()->id; } if (Request::submitted('store')) { - CSRFProtection::verifyRequest(); + CSRFProtection::verifyUnsafeRequest(); $this->group->name = Request::get('name'); $this->group->store(); $this->redirect('contact/index/' . $this->group->id); @@ -175,7 +175,7 @@ class ContactController extends AuthenticatedController public function deleteGroup_action() { - CSRFProtection::verifyRequest(); + CSRFProtection::verifyUnsafeRequest(); $this->group->delete(); $this->redirect('contact/index'); } diff --git a/app/controllers/course/timesrooms.php b/app/controllers/course/timesrooms.php index ecee7fc..bbc7196 100644 --- a/app/controllers/course/timesrooms.php +++ b/app/controllers/course/timesrooms.php @@ -1269,7 +1269,8 @@ class Course_TimesroomsController extends AuthenticatedController */ public function saveCycle_action() { - CSRFProtection::verifyRequest(); + CSRFProtection::verifyUnsafeRequest(); + $start = strtotime(Request::get('start_time')); $end = strtotime(Request::get('end_time')); @@ -1420,7 +1421,8 @@ class Course_TimesroomsController extends AuthenticatedController */ public function deleteCycle_action($cycle_id) { - CSRFProtection::verifyRequest(); + CSRFProtection::verifyUnsafeRequest(); + $cycle = SeminarCycleDate::find($cycle_id); if ($cycle === null) { $message = sprintf(_('Es gibt keinen regelmäßigen Eintrag "%s".'), $cycle_id); diff --git a/app/controllers/course/wiki.php b/app/controllers/course/wiki.php index cf87b08..647ab8a 100644 --- a/app/controllers/course/wiki.php +++ b/app/controllers/course/wiki.php @@ -299,9 +299,12 @@ class Course_WikiController extends AuthenticatedController public function delete_action(WikiPage $page) { - if (!Request::isPost() || !$page->isEditable() || !CSRFProtection::verifyRequest()) { + CSRFProtection::verifyUnsafeRequest(); + + if (!$page->isEditable()) { throw new AccessDeniedException(); } + $name = $page->name; $page->delete(); PageLayout::postSuccess(sprintf(_('Die Seite %s wurde gelöscht.'), htmlReady($name))); @@ -310,7 +313,9 @@ class Course_WikiController extends AuthenticatedController public function deleteversion_action(WikiPage $page) { - if (!Request::isPost() || !$page->isEditable() || !CSRFProtection::verifyRequest()) { + CSRFProtection::verifyUnsafeRequest(); + + if (!$page->isEditable()) { throw new AccessDeniedException(); } diff --git a/app/controllers/materialien/files.php b/app/controllers/materialien/files.php index fc8b2aa..25fd5b2 100644 --- a/app/controllers/materialien/files.php +++ b/app/controllers/materialien/files.php @@ -449,7 +449,7 @@ class Materialien_FilesController extends MVVController public function delete_range_action($mvvfile_id, $range_id) { - CSRFProtection::verifyRequest(); + CSRFProtection::verifyUnsafeRequest(); if ($mvvfile_range = MvvFileRange::find([$mvvfile_id, $range_id])) { $vacant = $mvvfile_range->position; @@ -474,36 +474,9 @@ class Materialien_FilesController extends MVVController } } - public function delete_fileref_action($mvvfile_id, $fileref_id) - { - CSRFProtection::verifyRequest(); - - if ($mvv_file = MvvFile::find($mvvfile_id)) { - $vacant = $mvv_file->position; - $range_id = $mvv_file->range_id; - if ($mvv_file->delete()) { - foreach (MvvFile::findBySQL('range_id = ? ORDER BY position ASC',[$range_id]) as $other_file) { - if ($other_file->position > $vacant) { - $tmp = $other_file->position; - $other_file->position = $vacant; - $other_file->store(); - $vacant = $tmp; - } - } - PageLayout::postSuccess(_('Das Dokument wurde gelöscht.')); - } - } - $this->range_id = $range_id; - if (Request::isXhr()) { - $this->response->add_header('X-Dialog-Execute', 'STUDIP.MVV.Document.reload_documenttable("' . $range_id . '")'); - $this->response->add_header('X-Dialog-Close', 1); - $this->render_nothing(); - } - } - public function delete_all_dokument_action($mvvfile_id) { - CSRFProtection::verifyRequest(); + CSRFProtection::verifyUnsafeRequest(); MvvFile::deleteBySQL('mvvfile_id =?', [$mvvfile_id]); MvvFileRange::deleteBySQL('mvvfile_id =?', [$mvvfile_id]); diff --git a/app/controllers/questionnaire.php b/app/controllers/questionnaire.php index fa90ac2..c136178 100644 --- a/app/controllers/questionnaire.php +++ b/app/controllers/questionnaire.php @@ -379,9 +379,12 @@ class QuestionnaireController extends AuthenticatedController public function reset_action(Questionnaire $questionnaire) { - if (!Request::isPost() || !$questionnaire->isEditable() || !CSRFProtection::verifyRequest()) { + CSRFProtection::verifyUnsafeRequest(); + + if (!$questionnaire->isEditable()) { throw new AccessDeniedException(); } + foreach ($questionnaire->anonymousanswers as $anonymous) { $anonymous->delete(); } diff --git a/app/controllers/settings/deputies.php b/app/controllers/settings/deputies.php index 3476692..8ce4555 100644 --- a/app/controllers/settings/deputies.php +++ b/app/controllers/settings/deputies.php @@ -101,7 +101,7 @@ class Settings_DeputiesController extends Settings_SettingsController public function add_member_action() { - CSRFProtection::verifyRequest(); + CSRFProtection::verifyUnsafeRequest(); $mp = MultiPersonSearch::load('settings_add_deputy'); $msg = [ diff --git a/app/controllers/shared/contacts.php b/app/controllers/shared/contacts.php index 820d2ab..0cce44f 100644 --- a/app/controllers/shared/contacts.php +++ b/app/controllers/shared/contacts.php @@ -641,7 +641,7 @@ class Shared_ContactsController extends MVVController public function delete_all_ranges_action($contact_id = null) { - CSRFProtection::verifyRequest(); + CSRFProtection::verifyUnsafeRequest(); $contact = MvvContact::find($contact_id); if (!($contact && MvvPerm::get($contact)->haveFieldPerm('ranges', MvvPerm::PERM_CREATE))) { @@ -662,7 +662,7 @@ class Shared_ContactsController extends MVVController public function delete_extern_contact_action($user_id = null) { - CSRFProtection::verifyRequest(); + CSRFProtection::verifyUnsafeRequest(); if ($mvv_ext_contact = MvvExternContact::find($user_id)) { $mvv_ext_contact->delete(); diff --git a/app/controllers/studiengaenge/studiengaenge.php b/app/controllers/studiengaenge/studiengaenge.php index 8465665..44f5d83 100644 --- a/app/controllers/studiengaenge/studiengaenge.php +++ b/app/controllers/studiengaenge/studiengaenge.php @@ -288,17 +288,13 @@ class Studiengaenge_StudiengaengeController extends MVVController $studiengang = Studiengang::find($studiengang_id); if (!$studiengang) { PageLayout::postError(_('Unbekannter Studiengang')); - } else { - if (Request::isPost()) { - if (Request::submitted('delete')) { - CSRFProtection::verifyRequest(); - PageLayout::postSuccess(sprintf( - _('Studiengang "%s" gelöscht!'), - htmlReady($studiengang->name) - )); - $studiengang->delete(); - } - } + } else if (Request::submitted('delete')) { + CSRFProtection::verifyUnsafeRequest(); + PageLayout::postSuccess(sprintf( + _('Studiengang "%s" gelöscht!'), + htmlReady($studiengang->name) + )); + $studiengang->delete(); } $this->redirect($this->action_url('index')); } @@ -538,23 +534,21 @@ class Studiengaenge_StudiengaengeController extends MVVController if ($this->stg_stgteil->isNew()) { PageLayout::postError(_('Unbekannter Studiengangteil')); } else { - if (Request::isPost()) { - CSRFProtection::verifyRequest(); - if (!MvvPerm::haveFieldPermStudiengangteile($studiengang, MvvPerm::PERM_CREATE)) { - throw new Trails\Exception(403); - } - $stgteil_name = $this->stg_stgteil->stgteil_name; - $stgbez_name = $this->stg_stgteil->stgbez_name; - if ($this->stg_stgteil->delete()) { - PageLayout::postSuccess(sprintf( - _('Die Zuordnung des Studiengangteils "%s" als "%s" zum Studiengang "%s" wurde gelöscht.'), - htmlReady($stgteil_name), - htmlReady($stgbez_name), - htmlReady($studiengang->name) - )); - } else { - PageLayout::postError(_('Der Studiengangteil konnte nicht gelöscht werden.')); - } + CSRFProtection::verifyUnsafeRequest(); + if (!MvvPerm::haveFieldPermStudiengangteile($studiengang, MvvPerm::PERM_CREATE)) { + throw new Trails\Exception(403); + } + $stgteil_name = $this->stg_stgteil->stgteil_name; + $stgbez_name = $this->stg_stgteil->stgbez_name; + if ($this->stg_stgteil->delete()) { + PageLayout::postSuccess(sprintf( + _('Die Zuordnung des Studiengangteils "%s" als "%s" zum Studiengang "%s" wurde gelöscht.'), + htmlReady($stgteil_name), + htmlReady($stgbez_name), + htmlReady($studiengang->name) + )); + } else { + PageLayout::postError(_('Der Studiengangteil konnte nicht gelöscht werden.')); } } $this->redirect( diff --git a/app/views/materialien/files/index.php b/app/views/materialien/files/index.php index 95ceac9..dc3bf7e 100644 --- a/app/views/materialien/files/index.php +++ b/app/views/materialien/files/index.php @@ -1,123 +1,128 @@ - - - - - renderSortLink('materialien/files/', _('Name'), 'mvv_files_filerefs.name') ?> - renderSortLink('materialien/files/', _('Dateiname'), 'file_refs.name') ?> - renderSortLInk('materialien/files/', _('Sichtbarkeit'), 'extern_visible') ?> - renderSortLInk('materialien/files/', _('Sprache'), 'file_language') ?> - renderSortLink('materialien/files/', _('Art der Datei'), 'type') ?> - renderSortLink('materialien/files/', _('Datum'), 'mkdate') ?> - - renderSortLInk('materialien/files/', _('Kategorie'), 'category') ?> - renderSortLink('materialien/files/', _('Zuordnungen'), 'count_relations') ?> - - - - - - - - + + getRangesArray()) && ($fileref_id && in_array($fileref_id, $mvv_file->getFileRefsArray()))) : ?> + + render_partial('materialien/files/details', compact('mvv_file')) ?> + + + + + MVVController::$items_per_page) : ?> + + + + + + +
- - -
- - getDisplayName()) ?> - - - getFiletypes()[0] === 'Link'): ?> - - asImg(['class' => 'text-bottom']) ?> +
+ + + + + + renderSortLink('materialien/files/', _('Name'), 'mvv_files_filerefs.name') ?> + renderSortLink('materialien/files/', _('Dateiname'), 'file_refs.name') ?> + renderSortLInk('materialien/files/', _('Sichtbarkeit'), 'extern_visible') ?> + renderSortLInk('materialien/files/', _('Sprache'), 'file_language') ?> + renderSortLink('materialien/files/', _('Art der Datei'), 'type') ?> + renderSortLink('materialien/files/', _('Datum'), 'mkdate') ?> + + renderSortLInk('materialien/files/', _('Kategorie'), 'category') ?> + renderSortLink('materialien/files/', _('Zuordnungen'), 'count_relations') ?> + + + + + + + + - - - - - - - - + + + + + + + + - - getRangesArray()) && ($fileref_id && in_array($fileref_id, $mvv_file->getFileRefsArray()))) : ?> - - render_partial('materialien/files/details', compact('mvv_file')) ?> - - - - - MVVController::$items_per_page) : ?> - - - - - - -
+ + +
+ + getDisplayName()) ?> + + + getFiletypes()[0] === 'Link'): ?> + + asImg(['class' => 'text-bottom']) ?> + getFilenames()[0]); ?> + + getFilenames()[0]); ?> - - - getFilenames()[0]); ?> - - - extern_visible?'visibility-visible':'visibility-invisible', - Icon::ROLE_INFO - )->asImg([ - 'class' => 'text-bottom', - 'title' => $mvv_file->extern_visible?_('sichtbar'):_('unsichtbar') - ]) ?> - - file_refs as $fileref) : ?> - file_language), ['size' => '24']) ?> - - type]['name']) ?>mkdate) ?>getFiletypes()[0]) ?>category]['name']) ?>count_relations) ?> - setContext($mvv_file->getFilenames()[0]); - $actions->addLink( - $controller->url_for('materialien/files/add_dokument', 'index', $mvv_file->getRangeType()?:0, 0, $mvv_file->mvvfile_id), - _('Dokument bearbeiten'), - Icon::create('edit'), - ['data-dialog' => 'size=auto'] - ); - $actions->addLink( - $controller->url_for('materialien/files/add_ranges_to_file',$mvv_file->mvvfile_id), - _('Dokument zuordnen'), - Icon::create('add'), - ['data-dialog' => 'size=auto'] - ); - foreach ($mvv_file->file_refs as $fileref) { + + + extern_visible?'visibility-visible':'visibility-invisible', + Icon::ROLE_INFO + )->asImg([ + 'class' => 'text-bottom', + 'title' => $mvv_file->extern_visible?_('sichtbar'):_('unsichtbar') + ]) ?> + + file_refs as $fileref) : ?> + file_language), ['size' => '24']) ?> + + type]['name']) ?>mkdate) ?>getFiletypes()[0]) ?>category]['name']) ?>count_relations) ?> + setContext($mvv_file->getFilenames()[0]); $actions->addLink( - $fileref->file_ref->getDownloadURL('force_download'), - _('Datei herunterladen') . ' (' . $fileref->file_language . ')', - Icon::create('download'), - ['target' => '_blank'] + $controller->url_for('materialien/files/add_dokument', 'index', $mvv_file->getRangeType()?:0, 0, $mvv_file->mvvfile_id), + _('Dokument bearbeiten'), + Icon::create('edit'), + ['data-dialog' => 'size=auto'] ); - } - $actions->addLink( - $controller->url_for("materialien/files/delete_all_dokument/{$mvv_file->mvvfile_id}"), - _('Dokument löschen'), - Icon::create('trash'), - [ - 'data-confirm' => _('Wollen Sie das Dokument wirklich entfernen?'), - 'data-dialog' => 'size=auto' - ] - ); - echo $actions->render(); - ?> -
- open('shared/pagechooser'); - $pagination->clear_attributes(); - $pagination->set_attribute('perPage', MVVController::$items_per_page); - $pagination->set_attribute('num_postings', $count); - $pagination->set_attribute('page', $page); - // ARGH! - $page_link = explode('?', $controller->action_url('index'))[0] . '?page_files=%s'; - $pagination->set_attribute('pagelink', $page_link); - echo $pagination->render(); + $actions->addLink( + $controller->url_for('materialien/files/add_ranges_to_file',$mvv_file->mvvfile_id), + _('Dokument zuordnen'), + Icon::create('add'), + ['data-dialog' => 'size=auto'] + ); + foreach ($mvv_file->file_refs as $fileref) { + $actions->addLink( + $fileref->file_ref->getDownloadURL('force_download'), + _('Datei herunterladen') . ' (' . $fileref->file_language . ')', + Icon::create('download'), + ['target' => '_blank'] + ); + } + $actions->addButton( + 'delete', + _('Dokument löschen'), + Icon::create('trash'), + [ + 'data-confirm' => _('Wollen Sie das Dokument wirklich entfernen?'), + 'data-dialog' => 'size=auto', + 'formaction' => $controller->url_for("materialien/files/delete_all_dokument/{$mvv_file->mvvfile_id}"), + + ] + ); + echo $actions->render(); ?> -
+
+ open('shared/pagechooser'); + $pagination->clear_attributes(); + $pagination->set_attribute('perPage', MVVController::$items_per_page); + $pagination->set_attribute('num_postings', $count); + $pagination->set_attribute('page', $page); + // ARGH! + $page_link = explode('?', $controller->action_url('index'))[0] . '?page_files=%s'; + $pagination->set_attribute('pagelink', $page_link); + echo $pagination->render(); + ?> +
+