From 944c29b96429ec95ac1371cb33cc43704a60c7b1 Mon Sep 17 00:00:00 2001 From: Igor Scheller 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/pages/admin_active.php | 133 +++++++++++++++++++------------------ includes/pages/admin_arrive.php | 33 +++++---- includes/pages/admin_groups.php | 6 +- includes/pages/admin_import.php | 2 +- includes/pages/admin_questions.php | 28 ++++---- includes/pages/admin_rooms.php | 14 ++-- includes/pages/admin_shifts.php | 2 +- includes/pages/guest_login.php | 4 +- includes/pages/user_myshifts.php | 2 +- includes/pages/user_news.php | 2 +- includes/pages/user_questions.php | 8 ++- includes/pages/user_settings.php | 8 +-- 12 files changed, 128 insertions(+), 114 deletions(-) (limited to 'includes/pages') diff --git a/includes/pages/admin_active.php b/includes/pages/admin_active.php index 9bd854c9..20f11a31 100644 --- a/includes/pages/admin_active.php +++ b/includes/pages/admin_active.php @@ -49,7 +49,7 @@ function admin_active() redirect(page_link_to('admin_active')); } - if ($request->has('ack')) { + if ($request->hasPostData('ack')) { State::query() ->where('got_shirt', '=', false) ->where('got_shirt', '=', false) @@ -94,61 +94,58 @@ function admin_active() $msg = success(__('Marked angels.'), true); } else { - $set_active = '« ' - . __('back') - . ' | ' - . __('apply') - . ''; + $set_active = form([ + button(page_link_to('admin_active', ['search' => $search]), '« ' . __('back')), + form_submit('ack', '» ' . __('apply')), + ], page_link_to('admin_active', ['search' => $search, 'count' => $count, 'set_active' => 1])); } } - if ($request->has('active') && preg_match('/^\d+$/', $request->input('active'))) { - $user_id = $request->input('active'); - $user_source = User::find($user_id); - if ($user_source) { - $user_source->state->active = true; - $user_source->state->save(); - engelsystem_log('User ' . User_Nick_render($user_source) . ' is active now.'); - $msg = success(__('Angel has been marked as active.'), true); - } else { - $msg = error(__('Angel not found.'), true); - } - } elseif ($request->has('not_active') && preg_match('/^\d+$/', $request->input('not_active'))) { - $user_id = $request->input('not_active'); - $user_source = User::find($user_id); - if ($user_source) { - $user_source->state->active = false; - $user_source->state->save(); - engelsystem_log('User ' . User_Nick_render($user_source) . ' is NOT active now.'); - $msg = success(__('Angel has been marked as not active.'), true); - } else { - $msg = error(__('Angel not found.'), true); - } - } elseif ($request->has('tshirt') && preg_match('/^\d+$/', $request->input('tshirt'))) { - $user_id = $request->input('tshirt'); - $user_source = User::find($user_id); - if ($user_source) { - $user_source->state->got_shirt = true; - $user_source->state->save(); - engelsystem_log('User ' . User_Nick_render($user_source) . ' has tshirt now.'); - $msg = success(__('Angel has got a t-shirt.'), true); - } else { - $msg = error('Angel not found.', true); - } - } elseif ($request->has('not_tshirt') && preg_match('/^\d+$/', $request->input('not_tshirt'))) { - $user_id = $request->input('not_tshirt'); - $user_source = User::find($user_id); - if ($user_source) { - $user_source->state->got_shirt = false; - $user_source->state->save(); - engelsystem_log('User ' . User_Nick_render($user_source) . ' has NO tshirt.'); - $msg = success(__('Angel has got no t-shirt.'), true); - } else { - $msg = error(__('Angel not found.'), true); + if ($request->hasPostData('submit')) { + if ($request->has('active') && preg_match('/^\d+$/', $request->input('active'))) { + $user_id = $request->input('active'); + $user_source = User::find($user_id); + if ($user_source) { + $user_source->state->active = true; + $user_source->state->save(); + engelsystem_log('User ' . User_Nick_render($user_source) . ' is active now.'); + $msg = success(__('Angel has been marked as active.'), true); + } else { + $msg = error(__('Angel not found.'), true); + } + } elseif ($request->has('not_active') && preg_match('/^\d+$/', $request->input('not_active'))) { + $user_id = $request->input('not_active'); + $user_source = User::find($user_id); + if ($user_source) { + $user_source->state->active = false; + $user_source->state->save(); + engelsystem_log('User ' . User_Nick_render($user_source) . ' is NOT active now.'); + $msg = success(__('Angel has been marked as not active.'), true); + } else { + $msg = error(__('Angel not found.'), true); + } + } elseif ($request->has('tshirt') && preg_match('/^\d+$/', $request->input('tshirt'))) { + $user_id = $request->input('tshirt'); + $user_source = User::find($user_id); + if ($user_source) { + $user_source->state->got_shirt = true; + $user_source->state->save(); + engelsystem_log('User ' . User_Nick_render($user_source) . ' has tshirt now.'); + $msg = success(__('Angel has got a t-shirt.'), true); + } else { + $msg = error('Angel not found.', true); + } + } elseif ($request->has('not_tshirt') && preg_match('/^\d+$/', $request->input('not_tshirt'))) { + $user_id = $request->input('not_tshirt'); + $user_source = User::find($user_id); + if ($user_source) { + $user_source->state->got_shirt = false; + $user_source->state->save(); + engelsystem_log('User ' . User_Nick_render($user_source) . ' has NO tshirt.'); + $msg = success(__('Angel has got no t-shirt.'), true); + } else { + $msg = error(__('Angel not found.'), true); + } } } @@ -232,9 +229,10 @@ function admin_active() if ($show_all_shifts) { $parameters['show_all_shifts'] = 1; } - $actions[] = '' - . __('set active') - . ''; + $actions[] = form( + [form_submit('submit', __('set active'), 'btn-xs', false)], + page_link_to('admin_active', $parameters) + ); } if ($usr->state->active) { $parametersRemove = [ @@ -244,9 +242,10 @@ function admin_active() if ($show_all_shifts) { $parametersRemove['show_all_shifts'] = 1; } - $actions[] = '' - . __('remove active') - . ''; + $actions[] = form( + [form_submit('submit', __('remove active'), 'btn-xs', false)], + page_link_to('admin_active', $parametersRemove) + ); } if (!$usr->state->got_shirt) { $parametersShirt = [ @@ -256,9 +255,10 @@ function admin_active() if ($show_all_shifts) { $parametersShirt['show_all_shifts'] = 1; } - $actions[] = '' - . __('got t-shirt') - . ''; + $actions[] = form( + [form_submit('submit', __('got t-shirt'), 'btn-xs', false)], + page_link_to('admin_active', $parametersShirt) + ); } if ($usr->state->got_shirt) { $parameters = [ @@ -268,12 +268,13 @@ function admin_active() if ($show_all_shifts) { $parameters['show_all_shifts'] = 1; } - $actions[] = '' - . __('remove t-shirt') - . ''; + $actions[] = form( + [form_submit('submit', __('remove t-shirt'), 'btn-xs', false)], + page_link_to('admin_active', $parameters) + ); } - $userData['actions'] = join(' ', $actions); + $userData['actions'] = buttons($actions); $matched_users[] = $userData; } diff --git a/includes/pages/admin_arrive.php b/includes/pages/admin_arrive.php index 2b4d7a3f..0714d980 100644 --- a/includes/pages/admin_arrive.php +++ b/includes/pages/admin_arrive.php @@ -24,8 +24,13 @@ function admin_arrive() $search = trim($search); } - if ($request->has('reset') && preg_match('/^\d+$/', $request->input('reset'))) { - $user_id = $request->input('reset'); + $action = $request->get('action'); + if ( + $action == 'reset' + && preg_match('/^\d+$/', $request->input('user')) + && $request->hasPostData('submit') + ) { + $user_id = $request->input('user'); $user_source = User::find($user_id); if ($user_source) { $user_source->state->arrived = false; @@ -38,8 +43,12 @@ function admin_arrive() } else { $msg = error(__('Angel not found.'), true); } - } elseif ($request->has('arrived') && preg_match('/^\d+$/', $request->input('arrived'))) { - $user_id = $request->input('arrived'); + } elseif ( + $action == 'arrived' + && preg_match('/^\d+$/', $request->input('user')) + && $request->hasPostData('submit') + ) { + $user_id = $request->input('user'); $user_source = User::find($user_id); if ($user_source) { $user_source->state->arrived = true; @@ -88,15 +97,11 @@ function admin_arrive() $usr['rendered_planned_arrival_date'] = $plannedArrivalDate ? $plannedArrivalDate->format('Y-m-d') : '-'; $usr['rendered_arrival_date'] = $arrivalDate ? $arrivalDate->format('Y-m-d') : '-'; $usr['arrived'] = $usr->state->arrived ? __('yes') : ''; - $usr['actions'] = $usr->state->arrived == 1 - ? '' . __('reset') . '' - : '' . __('arrived') . ''; + $usr['actions'] = form([ + form_hidden('action', $usr->state->arrived ? 'reset' : 'arrived'), + form_hidden('user', $usr->id), + form_submit('submit', $usr->state->arrived ? __('reset') : __('arrived'), 'btn-xs'), + ]); if ($usr->state->arrival_date) { $day = $usr->state->arrival_date->format('Y-m-d'); @@ -167,7 +172,7 @@ function admin_arrive() form([ form_text('search', __('Search'), $search), form_submit('submit', __('Search')) - ]), + ], page_link_to('admin_arrive')), table([ 'name' => __('Nickname'), 'rendered_planned_arrival_date' => __('Planned arrival'), diff --git a/includes/pages/admin_groups.php b/includes/pages/admin_groups.php index 727d7be5..ca6aba72 100644 --- a/includes/pages/admin_groups.php +++ b/includes/pages/admin_groups.php @@ -110,7 +110,11 @@ function admin_groups() break; case 'save': - if ($request->has('id') && preg_match('/^-\d{1,11}$/', $request->input('id'))) { + if ( + $request->has('id') + && preg_match('/^-\d{1,11}$/', $request->input('id')) + && $request->hasPostData('submit') + ) { $group_id = $request->input('id'); } else { return error('Incomplete call, missing Groups ID.', true); diff --git a/includes/pages/admin_import.php b/includes/pages/admin_import.php index 3ca9cb65..11c9729a 100644 --- a/includes/pages/admin_import.php +++ b/includes/pages/admin_import.php @@ -54,7 +54,7 @@ function admin_import() case 'input': $valid = false; - if ($request->has('submit')) { + if ($request->hasPostData('submit')) { $valid = true; if ($request->has('shifttype_id') && isset($shifttypes[$request->input('shifttype_id')])) { diff --git a/includes/pages/admin_questions.php b/includes/pages/admin_questions.php index 4f0f0bfc..60df1ebf 100644 --- a/includes/pages/admin_questions.php +++ b/includes/pages/admin_questions.php @@ -56,11 +56,9 @@ function admin_questions() form_textarea('answer', '', ''), form_submit('submit', __('Save')) ], page_link_to('admin_questions', ['action' => 'answer', 'id' => $question['QID']])), - 'actions' => button( - page_link_to('admin_questions', ['action' => 'delete', 'id' => $question['QID']]), - __('delete'), - 'btn-xs' - ) + 'actions' => form([ + form_submit('submit', __('delete'), 'btn-xs'), + ], page_link_to('admin_questions', ['action' => 'delete', 'id' => $question['QID']])), ]; } @@ -74,11 +72,9 @@ function admin_questions() 'question' => str_replace("\n", '
', $question['Question']), 'answered_by' => User_Nick_render($answer_user_source), 'answer' => str_replace("\n", '
', $question['Answer']), - 'actions' => button( - page_link_to('admin_questions', ['action' => 'delete', 'id' => $question['QID']]), - __('delete'), - 'btn-xs' - ) + 'actions' => form([ + form_submit('submit', __('delete'), 'btn-xs') + ], page_link_to('admin_questions', ['action' => 'delete', 'id' => $question['QID']])) ]; } @@ -102,7 +98,11 @@ function admin_questions() } else { switch ($request->input('action')) { case 'answer': - if ($request->has('id') && preg_match('/^\d{1,11}$/', $request->input('id'))) { + if ( + $request->has('id') + && preg_match('/^\d{1,11}$/', $request->input('id')) + && $request->hasPostData('submit') + ) { $question_id = $request->input('id'); } else { return error('Incomplete call, missing Question ID.', true); @@ -142,7 +142,11 @@ function admin_questions() } break; case 'delete': - if ($request->has('id') && preg_match('/^\d{1,11}$/', $request->input('id'))) { + if ( + $request->has('id') + && preg_match('/^\d{1,11}$/', $request->input('id')) + && $request->hasPostData('submit') + ) { $question_id = $request->input('id'); } else { return error('Incomplete call, missing Question ID.', true); diff --git a/includes/pages/admin_rooms.php b/includes/pages/admin_rooms.php index 5be3f926..558145bc 100644 --- a/includes/pages/admin_rooms.php +++ b/includes/pages/admin_rooms.php @@ -72,7 +72,7 @@ function admin_rooms() } if ($request->input('show') == 'edit') { - if ($request->has('submit')) { + if ($request->hasPostData('submit')) { $valid = true; if ($request->has('name') && strlen(strip_request_item('name')) > 0) { @@ -178,7 +178,7 @@ function admin_rooms() ]) ]); } elseif ($request->input('show') == 'delete') { - if ($request->has('ack')) { + if ($request->hasPostData('ack')) { Room_delete($room_id); engelsystem_log('Room deleted: ' . $name); @@ -191,13 +191,9 @@ function admin_rooms() button(page_link_to('admin_rooms'), __('back'), 'back') ]), sprintf(__('Do you want to delete room %s?'), $name), - buttons([ - button( - page_link_to('admin_rooms', ['show' => 'delete', 'id' => $room_id, 'ack' => 1]), - __('Delete'), - 'delete btn-danger' - ) - ]) + form([ + form_submit('ack', __('Delete'), 'delete btn-danger'), + ], page_link_to('admin_rooms', ['show' => 'delete', 'id' => $room_id])), ]); } } diff --git a/includes/pages/admin_shifts.php b/includes/pages/admin_shifts.php index 171fa073..d697a496 100644 --- a/includes/pages/admin_shifts.php +++ b/includes/pages/admin_shifts.php @@ -307,7 +307,7 @@ function admin_shifts() ]) ]); } - } elseif ($request->has('submit')) { + } elseif ($request->hasPostData('submit')) { if ( !is_array($session->get('admin_shifts_shifts')) || !is_array($session->get('admin_shifts_types')) diff --git a/includes/pages/guest_login.php b/includes/pages/guest_login.php index 5c3193f8..e1c6dfa4 100644 --- a/includes/pages/guest_login.php +++ b/includes/pages/guest_login.php @@ -79,7 +79,7 @@ function guest_register() ]); } - if ($request->has('submit')) { + if ($request->hasPostData('submit')) { $valid = true; if ($request->has('nick') && strlen(User_validate_Nick($request->input('nick'))) > 1) { @@ -388,7 +388,7 @@ function guest_login() $session->remove('uid'); - if ($request->has('submit')) { + if ($request->hasPostData('submit')) { if ($request->has('nick') && strlen(User_validate_Nick($request->input('nick'))) > 0) { $nick = User_validate_Nick($request->input('nick')); $login_user = User::whereName($nick)->first(); diff --git a/includes/pages/user_myshifts.php b/includes/pages/user_myshifts.php index 7fa33518..1eab016d 100644 --- a/includes/pages/user_myshifts.php +++ b/includes/pages/user_myshifts.php @@ -77,7 +77,7 @@ function user_myshifts() $freeloaded = $shift['freeloaded']; $freeload_comment = $shift['freeload_comment']; - if ($request->has('submit')) { + if ($request->hasPostData('submit')) { $valid = true; if (in_array('user_shifts_admin', $privileges)) { $freeloaded = $request->has('freeloaded'); diff --git a/includes/pages/user_news.php b/includes/pages/user_news.php index d7e681a6..e101be6b 100644 --- a/includes/pages/user_news.php +++ b/includes/pages/user_news.php @@ -142,7 +142,7 @@ function user_news_comments() ) { $nid = $request->input('nid'); $news = DB::selectOne('SELECT * FROM `News` WHERE `ID`=? LIMIT 1', [$nid]); - if ($request->has('text')) { + if ($request->hasPostData('submit') && $request->has('text')) { $text = preg_replace( "/([^\p{L}\p{P}\p{Z}\p{N}\n]{1,})/ui", '', diff --git a/includes/pages/user_questions.php b/includes/pages/user_questions.php index 9027ada7..19999577 100644 --- a/includes/pages/user_questions.php +++ b/includes/pages/user_questions.php @@ -43,7 +43,7 @@ function user_questions() switch ($request->input('action')) { case 'ask': $question = strip_request_item_nl('question'); - if ($question != '') { + if ($question != '' && $request->hasPostData('submit')) { DB::insert(' INSERT INTO `Questions` (`UID`, `Question`) VALUES (?, ?) @@ -60,7 +60,11 @@ function user_questions() } break; case 'delete': - if ($request->has('id') && preg_match('/^\d{1,11}$/', $request->input('id'))) { + if ( + $request->has('id') + && preg_match('/^\d{1,11}$/', $request->input('id')) + && $request->hasPostData('submit') + ) { $question_id = $request->input('id'); } else { return error(__('Incomplete call, missing Question ID.'), true); diff --git a/includes/pages/user_settings.php b/includes/pages/user_settings.php index f833eec5..c39c0ef7 100644 --- a/includes/pages/user_settings.php +++ b/includes/pages/user_settings.php @@ -204,13 +204,13 @@ function user_settings() } $user_source = auth()->user(); - if ($request->has('submit')) { + if ($request->hasPostData('submit')) { $user_source = user_settings_main($user_source, $enable_tshirt_size, $tshirt_sizes); - } elseif ($request->has('submit_password')) { + } elseif ($request->hasPostData('submit_password')) { user_settings_password($user_source); - } elseif ($request->has('submit_theme')) { + } elseif ($request->hasPostData('submit_theme')) { $user_source = user_settings_theme($user_source, $themes); - } elseif ($request->has('submit_language')) { + } elseif ($request->hasPostData('submit_language')) { $user_source = user_settings_locale($user_source, $locales); } -- cgit v1.2.3-70-g09d2