From 944c29b96429ec95ac1371cb33cc43704a60c7b1 Mon Sep 17 00:00:00 2001
From: Igor Scheller <igor.scheller@igorshp.de>
Date: Tue, 20 Nov 2018 16:02:03 +0100
Subject: Require POST for sending forms

* Ensure that the form is submitted with a post request
* Replaced several links with forms

Closes #494 (Security Vulnerability)
---
 includes/controller/angeltypes_controller.php           |  4 ++--
 includes/controller/event_config_controller.php         |  2 +-
 includes/controller/shift_entries_controller.php        | 12 ++++++------
 includes/controller/shifts_controller.php               | 11 ++++++-----
 includes/controller/shifttypes_controller.php           |  4 ++--
 includes/controller/user_angeltypes_controller.php      | 14 +++++++-------
 includes/controller/user_driver_licenses_controller.php |  2 +-
 includes/controller/user_worklog_controller.php         |  8 ++++----
 includes/controller/users_controller.php                | 10 ++++++----
 9 files changed, 35 insertions(+), 32 deletions(-)

(limited to 'includes/controller')

diff --git a/includes/controller/angeltypes_controller.php b/includes/controller/angeltypes_controller.php
index 82cbf935..821d101a 100644
--- a/includes/controller/angeltypes_controller.php
+++ b/includes/controller/angeltypes_controller.php
@@ -86,7 +86,7 @@ function angeltype_delete_controller()
 
     $angeltype = load_angeltype();
 
-    if (request()->has('confirmed')) {
+    if (request()->hasPostData('delete')) {
         AngelType_delete($angeltype);
         success(sprintf(__('Angeltype %s deleted.'), AngelType_name_render($angeltype)));
         redirect(page_link_to('angeltypes'));
@@ -127,7 +127,7 @@ function angeltype_edit_controller()
         $angeltype = AngelType_new();
     }
 
-    if ($request->has('submit')) {
+    if ($request->hasPostData('submit')) {
         $valid = true;
 
         if (!$supporter_mode) {
diff --git a/includes/controller/event_config_controller.php b/includes/controller/event_config_controller.php
index 79c276e4..e9b27cba 100644
--- a/includes/controller/event_config_controller.php
+++ b/includes/controller/event_config_controller.php
@@ -35,7 +35,7 @@ function event_config_edit_controller()
     /** @var Carbon $teardown_end_date */
     $teardown_end_date = $config->get('teardown_end');
 
-    if ($request->has('submit')) {
+    if ($request->hasPostData('submit')) {
         $valid = true;
 
         if ($request->has('event_name')) {
diff --git a/includes/controller/shift_entries_controller.php b/includes/controller/shift_entries_controller.php
index 95fbccfc..16f0c0a1 100644
--- a/includes/controller/shift_entries_controller.php
+++ b/includes/controller/shift_entries_controller.php
@@ -96,7 +96,7 @@ function shift_entry_create_controller_admin($shift, $angeltype)
         $angeltype = $angeltypes[0];
     }
 
-    if ($request->has('submit')) {
+    if ($request->hasPostData('submit')) {
         ShiftEntry_create([
             'SID'              => $shift['SID'],
             'TID'              => $angeltype['id'],
@@ -167,7 +167,7 @@ function shift_entry_create_controller_supporter($shift, $angeltype)
         redirect(shift_link($shift));
     }
 
-    if ($request->has('submit')) {
+    if ($request->hasPostData('submit')) {
         ShiftEntry_create([
             'SID'              => $shift['SID'],
             'TID'              => $angeltype['id'],
@@ -246,7 +246,7 @@ function shift_entry_create_controller_user($shift, $angeltype)
     }
 
     $comment = '';
-    if ($request->has('submit')) {
+    if ($request->hasPostData('submit')) {
         $comment = strip_request_item_nl('comment');
         ShiftEntry_create([
             'SID'              => $shift['SID'],
@@ -346,7 +346,7 @@ function shift_entry_delete_controller()
         redirect(user_link($signout_user->id));
     }
 
-    if ($request->has('continue')) {
+    if ($request->hasPostData('delete')) {
         ShiftEntry_delete($shiftEntry);
         success(__('Shift entry removed.'));
         redirect(shift_link($shift));
@@ -355,13 +355,13 @@ function shift_entry_delete_controller()
     if ($user->id == $signout_user->id) {
         return [
             ShiftEntry_delete_title(),
-            ShiftEntry_delete_view($shiftEntry, $shift, $angeltype, $signout_user->id)
+            ShiftEntry_delete_view($shift, $angeltype, $signout_user->id)
         ];
     }
 
     return [
         ShiftEntry_delete_title(),
-        ShiftEntry_delete_view_admin($shiftEntry, $shift, $angeltype, $signout_user)
+        ShiftEntry_delete_view_admin($shift, $angeltype, $signout_user)
     ];
 }
 
diff --git a/includes/controller/shifts_controller.php b/includes/controller/shifts_controller.php
index ee6714d4..375ea6b6 100644
--- a/includes/controller/shifts_controller.php
+++ b/includes/controller/shifts_controller.php
@@ -81,7 +81,7 @@ function shift_edit_controller()
     $start = $shift['start'];
     $end = $shift['end'];
 
-    if ($request->has('submit')) {
+    if ($request->hasPostData('submit')) {
         // Name/Bezeichnung der Schicht, darf leer sein
         $title = strip_request_item('title');
 
@@ -222,7 +222,7 @@ function shift_delete_controller()
     }
 
     // Schicht löschen bestätigt
-    if ($request->has('delete')) {
+    if ($request->hasPostData('delete')) {
         Shift_delete($shift_id);
 
         engelsystem_log(
@@ -241,9 +241,10 @@ function shift_delete_controller()
             date('Y-m-d H:i', $shift['start']),
             date('H:i', $shift['end'])
         ), true),
-        '<a class="button" href="'
-        . page_link_to('user_shifts', ['delete_shift' => $shift_id, 'delete' => 1]) .
-        '">' . __('delete') . '</a>'
+        form([
+            form_hidden('delete_shift', $shift_id),
+            form_submit('delete', __('delete')),
+        ]),
     ]);
 }
 
diff --git a/includes/controller/shifttypes_controller.php b/includes/controller/shifttypes_controller.php
index 8b30ea60..3c825d0c 100644
--- a/includes/controller/shifttypes_controller.php
+++ b/includes/controller/shifttypes_controller.php
@@ -26,7 +26,7 @@ function shifttype_delete_controller()
         redirect(page_link_to('shifttypes'));
     }
 
-    if ($request->has('confirmed')) {
+    if ($request->hasPostData('delete')) {
         ShiftType_delete($shifttype['id']);
 
         engelsystem_log('Deleted shifttype ' . $shifttype['name']);
@@ -67,7 +67,7 @@ function shifttype_edit_controller()
         $description = $shifttype['description'];
     }
 
-    if ($request->has('submit')) {
+    if ($request->hasPostData('submit')) {
         $valid = true;
 
         if ($request->has('name') && $request->input('name') != '') {
diff --git a/includes/controller/user_angeltypes_controller.php b/includes/controller/user_angeltypes_controller.php
index 4269313d..e03bd293 100644
--- a/includes/controller/user_angeltypes_controller.php
+++ b/includes/controller/user_angeltypes_controller.php
@@ -59,7 +59,7 @@ function user_angeltypes_delete_all_controller()
         redirect(page_link_to('angeltypes'));
     }
 
-    if ($request->has('confirmed')) {
+    if ($request->hasPostData('deny_all')) {
         UserAngelTypes_delete_all($angeltype['id']);
 
         engelsystem_log(sprintf('Denied all users for angeltype %s', AngelType_name_render($angeltype)));
@@ -100,7 +100,7 @@ function user_angeltypes_confirm_all_controller()
         redirect(page_link_to('angeltypes'));
     }
 
-    if ($request->has('confirmed')) {
+    if ($request->hasPostData('confirm_all')) {
         UserAngelTypes_confirm_all($angeltype['id'], $user->id);
 
         engelsystem_log(sprintf('Confirmed all users for angeltype %s', AngelType_name_render($angeltype)));
@@ -152,7 +152,7 @@ function user_angeltype_confirm_controller()
         redirect(page_link_to('angeltypes'));
     }
 
-    if ($request->has('confirmed')) {
+    if ($request->hasPostData('confirm_user')) {
         UserAngelType_confirm($user_angeltype['id'], $user->id);
 
         engelsystem_log(sprintf(
@@ -212,7 +212,7 @@ function user_angeltype_delete_controller()
         redirect(page_link_to('angeltypes'));
     }
 
-    if ($request->has('confirmed')) {
+    if ($request->hasPostData('delete')) {
         UserAngelType_delete($user_angeltype);
 
         $success_message = sprintf(__('User %s removed from %s.'), User_Nick_render($user_source), $angeltype['name']);
@@ -274,7 +274,7 @@ function user_angeltype_update_controller()
         redirect(page_link_to('angeltypes'));
     }
 
-    if ($request->has('confirmed')) {
+    if ($request->hasPostData('submit')) {
         UserAngelType_update($user_angeltype['id'], $supporter);
 
         $success_message = sprintf(
@@ -318,7 +318,7 @@ function user_angeltype_add_controller()
     // Load possible users, that are not in the angeltype already
     $users_source = Users_by_angeltype_inverted($angeltype);
 
-    if (request()->has('submit')) {
+    if (request()->hasPostData('submit')) {
         $user_source = load_user();
 
         if (!UserAngelType_exists($user_source->id, $angeltype)) {
@@ -369,7 +369,7 @@ function user_angeltype_join_controller($angeltype)
         redirect(page_link_to('angeltypes'));
     }
 
-    if (request()->has('confirmed')) {
+    if (request()->hasPostData('submit')) {
         $user_angeltype_id = UserAngelType_create($user->id, $angeltype);
 
         $success_message = sprintf(__('You joined %s.'), $angeltype['name']);
diff --git a/includes/controller/user_driver_licenses_controller.php b/includes/controller/user_driver_licenses_controller.php
index dd12db2a..69179b35 100644
--- a/includes/controller/user_driver_licenses_controller.php
+++ b/includes/controller/user_driver_licenses_controller.php
@@ -114,7 +114,7 @@ function user_driver_license_edit_controller()
         $wants_to_drive = true;
     }
 
-    if ($request->has('submit')) {
+    if ($request->hasPostData('submit')) {
         $wants_to_drive = $request->has('wants_to_drive');
         if ($wants_to_drive) {
             $user_driver_license['has_car'] = $request->has('has_car');
diff --git a/includes/controller/user_worklog_controller.php b/includes/controller/user_worklog_controller.php
index 333fd76e..4eaa5e91 100644
--- a/includes/controller/user_worklog_controller.php
+++ b/includes/controller/user_worklog_controller.php
@@ -16,7 +16,7 @@ function user_worklog_delete_controller()
     }
     $user_source = User::find($userWorkLog['user_id']);
 
-    if ($request->has('confirmed')) {
+    if ($request->hasPostData('submit')) {
         UserWorkLog_delete($userWorkLog);
 
         success(__('Work log entry deleted.'));
@@ -25,7 +25,7 @@ function user_worklog_delete_controller()
 
     return [
         UserWorkLog_delete_title(),
-        UserWorkLog_delete_view($user_source, $userWorkLog)
+        UserWorkLog_delete_view($user_source)
     ];
 }
 
@@ -43,7 +43,7 @@ function user_worklog_edit_controller()
     }
     $user_source = User::find($userWorkLog['user_id']);
 
-    if ($request->has('submit')) {
+    if ($request->hasPostData('submit')) {
         list ($valid, $userWorkLog) = user_worklog_from_request($userWorkLog);
 
         if ($valid) {
@@ -114,7 +114,7 @@ function user_worklog_add_controller()
 
     $userWorkLog = UserWorkLog_new($user_source->id);
 
-    if ($request->has('submit')) {
+    if ($request->hasPostData('submit')) {
         list ($valid, $userWorkLog) = user_worklog_from_request($userWorkLog);
 
         if ($valid) {
diff --git a/includes/controller/users_controller.php b/includes/controller/users_controller.php
index 0bf612d5..51b6e432 100644
--- a/includes/controller/users_controller.php
+++ b/includes/controller/users_controller.php
@@ -66,7 +66,7 @@ function user_delete_controller()
         redirect(user_link($user->id));
     }
 
-    if ($request->has('submit')) {
+    if ($request->hasPostData('submit')) {
         $valid = true;
 
         if (
@@ -80,6 +80,8 @@ function user_delete_controller()
         }
 
         if ($valid) {
+            // Load data before user deletion to prevent errors when displaying
+            $user_source->load(['contact', 'personalData', 'settings', 'state']);
             $user_source->delete();
 
             mail_user_delete($user_source);
@@ -150,7 +152,7 @@ function user_edit_vouchers_controller()
         redirect(page_link_to(''));
     }
 
-    if ($request->has('submit')) {
+    if ($request->hasPostData('submit')) {
         $valid = true;
 
         $vouchers = '';
@@ -326,7 +328,7 @@ function user_password_recovery_set_new_controller()
         redirect(page_link_to('login'));
     }
 
-    if ($request->has('submit')) {
+    if ($request->hasPostData('submit')) {
         $valid = true;
 
         if (
@@ -361,7 +363,7 @@ function user_password_recovery_set_new_controller()
 function user_password_recovery_start_controller()
 {
     $request = request();
-    if ($request->has('submit')) {
+    if ($request->hasPostData('submit')) {
         $valid = true;
 
         $user_source = null;
-- 
cgit v1.2.3-54-g00ecf