From afc564d342e7b915281af0f42580fbdc94a9077b Mon Sep 17 00:00:00 2001 From: Ignacio Nelson Date: Wed, 5 Jan 2022 23:51:09 -0300 Subject: [PATCH] Batch actions are sent as post instead of get to prevent malicious users from sending an action url to an admin user --- actions-log.php | 84 +++---- categories.php | 47 ++-- clients-membership-requests.php | 2 +- clients-requests.php | 2 +- clients.php | 120 +++++----- email-templates.php | 2 +- email-test.php | 2 +- groups.php | 81 +++---- import-orphans.php | 2 +- includes/forms/categories.php | 2 +- includes/forms/clients.php | 2 +- includes/forms/file_editor.php | 2 +- includes/forms/groups.php | 2 +- includes/forms/reset-password/enter-email.php | 2 +- .../forms/reset-password/enter-password.php | 2 +- includes/forms/upload.php | 2 +- includes/forms/users.php | 2 +- includes/functions.forms.php | 33 +++ includes/functions.php | 6 + includes/security/csrf.php | 5 + manage-files.php | 214 +++++++++--------- options.php | 2 +- users.php | 48 ++-- 23 files changed, 356 insertions(+), 310 deletions(-) diff --git a/actions-log.php b/actions-log.php index 55abcdd01..be23213d7 100644 --- a/actions-log.php +++ b/actions-log.php @@ -14,50 +14,50 @@ $page_title = __('Recent activities log','cftp_admin'); include_once ADMIN_VIEWS_DIR . DS . 'header.php'; -?> -
-
- prepare("DELETE FROM " . TABLE_LOG . " WHERE FIND_IN_SET(id, :delete)"); - $params = array( - ':delete' => $delete_ids, - ); - $statement->execute( $params ); - - $msg = __('The selected activities were deleted.','cftp_admin'); - echo system_message('success',$msg); - } - else { - $msg = __('Please select at least one activity.','cftp_admin'); - echo system_message('danger',$msg); - } - break; - case 'log_clear': - $keep = '5,6,7,8,37'; - $statement = $dbh->prepare("DELETE FROM " . TABLE_LOG . " WHERE NOT ( FIND_IN_SET(action, :keep) ) "); - $params = array( - ':keep' => $keep, - ); - $statement->execute( $params ); + if ( !empty( $_POST['batch'] ) ) { + $statement = $dbh->prepare("DELETE FROM " . TABLE_LOG . " WHERE FIND_IN_SET(id, :delete)"); + $params = array( + ':delete' => $delete_ids, + ); + $statement->execute( $params ); - $msg = __('The log was cleared. Only data used for statistics remained. You can delete them manually if you want.','cftp_admin'); - echo system_message('success',$msg); - break; - } - } + $flash->success(__('The selected activities were deleted.', 'cftp_admin')); + } + else { + $flash->error(__('Please select at least one activity.', 'cftp_admin')); + } + break; + case 'log_clear': + $keep = '5,6,7,8,37'; + $statement = $dbh->prepare("DELETE FROM " . TABLE_LOG . " WHERE NOT ( FIND_IN_SET(action, :keep) ) "); + $params = array( + ':keep' => $keep, + ); + $statement->execute( $params ); + + $flash->success(__('The log was cleared. Only data used for statistics remained. You can delete them manually if you want.', 'cftp_admin')); + echo system_message('success',$msg); + break; + } + ps_redirect($current_url); +} +?> +
+
+
-
- + +
diff --git a/categories.php b/categories.php index 0bede10aa..e1c60e7cd 100644 --- a/categories.php +++ b/categories.php @@ -16,6 +16,7 @@ $page_id = 'categories_list'; include_once ADMIN_VIEWS_DIR . DS . 'header.php'; +$current_url = get_form_action_with_existing_parameters(basename(__FILE__)); /** * Messages set when adding or editing a category @@ -40,32 +41,32 @@ /** * Apply the corresponding action to the selected categories. */ -if ( isset( $_GET['action'] ) ) { - if ( $_GET['action'] != 'none' ) { +if ( isset( $_POST['action'] ) ) { + if ( $_POST['action'] != 'none' ) { /** Continue only if 1 or more categories were selected. */ - if ( !empty($_GET['batch'] ) ) { + if ( !empty($_POST['batch'] ) ) { /** * Make a list of categories to avoid individual queries. */ - $selected_categories = $_GET['batch']; + $selected_categories = $_POST['batch']; if (count($selected_categories) < 1 ) { - $msg = __('Please select at least one category.','cftp_admin'); - echo system_message('danger',$msg); - } - - switch($_GET['action']) { - case 'delete': - foreach ($selected_categories as $category_id) { - $category = new \ProjectSend\Classes\Categories(); - $category->get($category_id); - $delete_category = $category->delete(); - } - - $msg = __('The selected categories were deleted.','cftp_admin'); - echo system_message('success',$msg); + $flash->error(__('Please select at least one category.', 'cftp_admin')); + } else { + switch ($_POST['action']) { + case 'delete': + foreach ($selected_categories as $category_id) { + $category = new \ProjectSend\Classes\Categories(); + $category->get($category_id); + $delete_category = $category->delete(); + } + + $flash->success(__('The selected categories were deleted.', 'cftp_admin')); break; + } } + + ps_redirect($current_url); } } } @@ -184,8 +185,8 @@
- - + +
@@ -194,9 +195,9 @@ + diff --git a/clients-requests.php b/clients-requests.php index a9b92b8bd..1afb35c94 100644 --- a/clients-requests.php +++ b/clients-requests.php @@ -218,7 +218,7 @@
- +
diff --git a/clients.php b/clients.php index bfb7244c4..d381a916f 100644 --- a/clients.php +++ b/clients.php @@ -13,68 +13,68 @@ $page_title = __('Clients Administration','cftp_admin'); include_once ADMIN_VIEWS_DIR . DS . 'header.php'; -?> -
-
- get($work_client)) { - $hide_user = $this_client->setActiveStatus(1); - } - } - - $msg = __('The selected clients were marked as active.','cftp_admin'); - echo system_message('success',$msg); - break; - case 'deactivate': - /** - * Reverse of the previous action. Setting the value to 0 means - * that the client is inactive. - */ - foreach ($selected_clients as $work_client) { - $this_client = new \ProjectSend\Classes\Users(); - if ($this_client->get($work_client)) { - $hide_user = $this_client->setActiveStatus(0); - } - } - - $msg = __('The selected clients were marked as inactive.','cftp_admin'); - echo system_message('success',$msg); - break; - case 'delete': - foreach ($selected_clients as $work_client) { - $this_client = new \ProjectSend\Classes\Users(); - if ($this_client->get($work_client)) { - $delete_user = $this_client->delete(); - } - } - - $msg = __('The selected clients were deleted.','cftp_admin'); - echo system_message('success',$msg); - break; +$current_url = get_form_action_with_existing_parameters(basename(__FILE__)); + +/** + * Apply the corresponding action to the selected clients. + */ +if (isset($_POST['action'])) { + /** Continue only if 1 or more clients were selected. */ + if (!empty($_POST['batch'])) { + $selected_clients = $_POST['batch']; + + switch ($_POST['action']) { + case 'activate': + /** + * Changes the value on the "active" column value on the database. + * Inactive clients are not allowed to log in. + */ + foreach ($selected_clients as $work_client) { + $this_client = new \ProjectSend\Classes\Users(); + if ($this_client->get($work_client)) { + $hide_user = $this_client->setActiveStatus(1); + } + } + + $flash->success(__('The selected clients were marked as active.', 'cftp_admin')); + break; + case 'deactivate': + /** + * Reverse of the previous action. Setting the value to 0 means + * that the client is inactive. + */ + foreach ($selected_clients as $work_client) { + $this_client = new \ProjectSend\Classes\Users(); + if ($this_client->get($work_client)) { + $hide_user = $this_client->setActiveStatus(0); + } + } + + $flash->success(__('The selected clients were marked as inactive.', 'cftp_admin')); + break; + case 'delete': + foreach ($selected_clients as $work_client) { + $this_client = new \ProjectSend\Classes\Users(); + if ($this_client->get($work_client)) { + $delete_user = $this_client->delete(); + } } - } - else { - $msg = __('Please select at least one client.','cftp_admin'); - echo system_message('danger',$msg); - } + + $flash->success(__('The selected clients were deleted.', 'cftp_admin')); + break; } + } + else { + $flash->error(__('Please select at least one client.', 'cftp_admin')); + } + ps_redirect($current_url); +} +?> +
+
+
- - + +
diff --git a/email-templates.php b/email-templates.php index 03ecf2d22..281af55fa 100644 --- a/email-templates.php +++ b/email-templates.php @@ -343,7 +343,7 @@ - +
- + diff --git a/groups.php b/groups.php index 6332d8c1e..2e4323229 100644 --- a/groups.php +++ b/groups.php @@ -13,10 +13,12 @@ $page_title = __('Groups administration','cftp_admin'); +$current_url = get_form_action_with_existing_parameters(basename(__FILE__)); + /** * Used when viewing groups a certain client belongs to. */ -if(!empty($_GET['member'])) { +if (!empty($_GET['member'])) { $member = get_client_by_id($_GET['member']); /** Add the name of the client to the page's title. */ if ($member) { @@ -39,46 +41,45 @@ } } +/** + * Apply the corresponding action to the selected groups. + */ +if (isset($_POST['action']) && $_POST['action'] != 'none') { + /** Continue only if 1 or more groups were selected. */ + if (!empty($_POST['batch'])) { + $selected_groups = $_POST['batch']; + + switch ($_POST['action']) { + case 'delete': + $deleted_groups = 0; + + foreach ($selected_groups as $group) { + $this_group = new \ProjectSend\Classes\Groups; + if ($this_group->get($group)) { + $delete_user = $this_group->delete(); + $deleted_groups++; + } + } + + if ($deleted_groups > 0) { + $flash->success(__('The selected groups were deleted.', 'cftp_admin')); + } + break; + } + } + else { + $flash->error(__('Please select at least one group.', 'cftp_admin')); + } + + ps_redirect($current_url); +} + include_once ADMIN_VIEWS_DIR . DS . 'header.php'; ?>
get($group)) { - $delete_user = $this_group->delete(); - $deleted_groups++; - } - } - - if ($deleted_groups > 0) { - $msg = __('The selected groups were deleted.','cftp_admin'); - echo system_message('success',$msg); - } - break; - } - } - else { - $msg = __('Please select at least one group.','cftp_admin'); - echo system_message('danger',$msg); - } - } - $params = array(); $cq = "SELECT id FROM " . TABLE_GROUPS; @@ -141,8 +142,8 @@
- - + +
@@ -151,9 +152,9 @@ + - + diff --git a/includes/forms/clients.php b/includes/forms/clients.php index e0ee79ca0..364244f9f 100644 --- a/includes/forms/clients.php +++ b/includes/forms/clients.php @@ -70,7 +70,7 @@ ?> - +
diff --git a/includes/forms/file_editor.php b/includes/forms/file_editor.php index 6cf3534e3..d65054c75 100644 --- a/includes/forms/file_editor.php +++ b/includes/forms/file_editor.php @@ -1,5 +1,5 @@ - +
- +
diff --git a/includes/forms/reset-password/enter-email.php b/includes/forms/reset-password/enter-email.php index 435bd57ce..f025d9cbe 100644 --- a/includes/forms/reset-password/enter-email.php +++ b/includes/forms/reset-password/enter-email.php @@ -1,5 +1,5 @@ - +
diff --git a/includes/forms/reset-password/enter-password.php b/includes/forms/reset-password/enter-password.php index b01184197..396f85fe6 100644 --- a/includes/forms/reset-password/enter-password.php +++ b/includes/forms/reset-password/enter-password.php @@ -1,5 +1,5 @@ - +
diff --git a/includes/forms/upload.php b/includes/forms/upload.php index c9a911050..2687b477a 100644 --- a/includes/forms/upload.php +++ b/includes/forms/upload.php @@ -8,7 +8,7 @@ */ ?> - + diff --git a/includes/forms/users.php b/includes/forms/users.php index 86f2b85b1..f78b60a26 100644 --- a/includes/forms/users.php +++ b/includes/forms/users.php @@ -31,7 +31,7 @@ } ?> - +
diff --git a/includes/functions.forms.php b/includes/functions.forms.php index cbe6d62b7..3718051f4 100644 --- a/includes/functions.forms.php +++ b/includes/functions.forms.php @@ -44,6 +44,39 @@ function form_add_existing_parameters( $ignore = array() ) } } +/** + * Add any existing $_GET parameters to the form's action url + */ +function get_form_action_with_existing_parameters( $action = null, $ignore = array() ) +{ + $use = []; + + // Don't add the pagination parameter + $ignore[] = 'page'; + + // Remove this parameters so they only exist when the action is done + $remove = array('action', 'batch', 'status'); + + if ( !empty( $_GET ) ) { + foreach ( $_GET as $param => $value ) { + // Remove status and actions + if ( in_array( $param, $remove ) ) { + unset( $_GET[$param] ); + } + if ( !is_array( $value ) && !in_array( $param, $ignore ) ) { + $use[$param] = encode_html($value); + } + } + } + + $return = $action; + if (!empty($use)) { + $return .= '?' . http_build_query($use); + } + + return $return; +} + /** * Returns an existing or empty value to an input * diff --git a/includes/functions.php b/includes/functions.php index 21a81dc44..d284360b0 100644 --- a/includes/functions.php +++ b/includes/functions.php @@ -1894,4 +1894,10 @@ function recaptcha2ValidateRequest($redirect = true) } return $validation_passed; +} + +function ps_redirect($location, $status = 303) +{ + header("Location: $location", true, $status); + exit; } \ No newline at end of file diff --git a/includes/security/csrf.php b/includes/security/csrf.php index eceb3e814..61412d419 100644 --- a/includes/security/csrf.php +++ b/includes/security/csrf.php @@ -13,6 +13,11 @@ function getCsrfToken() return $_SESSION['csrf_token']; } +function addCsrf() +{ + echo ''; +} + /** * Validates the send csrf token with a stable string comparison algorithm. * Do not optimize for speed!!! diff --git a/manage-files.php b/manage-files.php index 47e8cba09..7224832f7 100644 --- a/manage-files.php +++ b/manage-files.php @@ -14,6 +14,8 @@ $page_id = 'manage_files'; +$current_url = get_form_action_with_existing_parameters(basename(__FILE__), array( 'modify_id', 'modify_type' )); + /** * Used to distinguish the current page results. * Global means all files. @@ -69,113 +71,113 @@ } } +/** + * Apply the corresponding action to the selected files. + */ +if (isset($_POST['action'])) { + /** Continue only if 1 or more files were selected. */ + if (!empty($_POST['batch'])) { + $selected_files = array_map('intval',array_unique($_POST['batch'])); + + switch ($_POST['action']) { + case 'hide': + /** + * Changes the value on the "hidden" column value on the database. + * This files are not shown on the client's file list. They are + * also not counted on the dashboard.php files count when the logged in + * account is the client. + */ + foreach ($selected_files as $file_id) { + $file = new \ProjectSend\Classes\Files; + $file->get($file_id); + $file->hide($results_type, $_POST['modify_id']); + } + + $flash->success(__('The selected files were marked as hidden.', 'cftp_admin')); + break; + case 'show': + foreach ($selected_files as $file_id) { + $file = new \ProjectSend\Classes\Files; + $file->get($file_id); + $file->show($results_type, $_POST['modify_id']); + } + + $flash->success(__('The selected files were marked as visible.', 'cftp_admin')); + break; + case 'hide_everyone': + foreach ($selected_files as $file_id) { + $file = new \ProjectSend\Classes\Files; + $file->get($file_id); + $file->hideFromEveryone(); + } + + $flash->success(__('The selected files were marked as hidden.', 'cftp_admin')); + break; + case 'show_everyone': + foreach ($selected_files as $file_id) { + $file = new \ProjectSend\Classes\Files; + $file->get($file_id); + $file->showToEveryone(); + } + + $flash->success(__('The selected files were marked as visible.', 'cftp_admin')); + break; + case 'unassign': + /** + * Remove the file from this client or group only. + */ + foreach ($selected_files as $file_id) { + $file = new \ProjectSend\Classes\Files; + $file->get($file_id); + $file->removeAssignment($results_type, $_POST['modify_id']); + } + + $flash->success(__('The selected files were successfully unassigned.', 'cftp_admin')); + break; + case 'delete': + $delete_results = array( + 'ok' => 0, + 'errors' => 0, + ); + foreach ($selected_files as $index => $file_id) { + if (!empty($file_id)) { + $file = new \ProjectSend\Classes\Files; + $file->get($file_id); + if ($file->deleteFiles()) { + $delete_results['ok']++; + } + else { + $delete_results['errors']++; + } + } + } + + if ( $delete_results['ok'] > 0 ) { + $flash->success(__('The selected files were deleted.', 'cftp_admin')); + } + if ( $delete_results['errors'] > 0 ) { + $flash->error(__('Some files could not be deleted.', 'cftp_admin')); + } + break; + case 'edit': + $url = BASE_URI.'files-edit.php?ids='.implode(',', $selected_files); + header("Location: ".$url); + exit; + break; + } + } + else { + $flash->error(__('Please select at least one file.', 'cftp_admin')); + } + + ps_redirect($current_url); +} + include_once ADMIN_VIEWS_DIR . DS . 'header.php'; ?>
get($file_id); - $file->hide($results_type, $_GET['modify_id']); - } - $msg = __('The selected files were marked as hidden.','cftp_admin'); - echo system_message('success',$msg); - break; - case 'show': - foreach ($selected_files as $file_id) { - $file = new \ProjectSend\Classes\Files; - $file->get($file_id); - $file->show($results_type, $_GET['modify_id']); - } - $msg = __('The selected files were marked as visible.','cftp_admin'); - echo system_message('success',$msg); - break; - case 'hide_everyone': - foreach ($selected_files as $file_id) { - $file = new \ProjectSend\Classes\Files; - $file->get($file_id); - $file->hideFromEveryone(); - } - $msg = __('The selected files were marked as hidden.','cftp_admin'); - echo system_message('success',$msg); - break; - case 'show_everyone': - foreach ($selected_files as $file_id) { - $file = new \ProjectSend\Classes\Files; - $file->get($file_id); - $file->showToEveryone(); - } - $msg = __('The selected files were marked as visible.','cftp_admin'); - echo system_message('success',$msg); - break; - case 'unassign': - /** - * Remove the file from this client or group only. - */ - foreach ($selected_files as $file_id) { - $file = new \ProjectSend\Classes\Files; - $file->get($file_id); - $file->removeAssignment($results_type, $_GET['modify_id']); - } - $msg = __('The selected files were successfully unassigned.','cftp_admin'); - echo system_message('success',$msg); - break; - case 'delete': - $delete_results = array( - 'ok' => 0, - 'errors' => 0, - ); - foreach ($selected_files as $index => $file_id) { - if (!empty($file_id)) { - $file = new \ProjectSend\Classes\Files; - $file->get($file_id); - if ($file->deleteFiles()) { - $delete_results['ok']++; - } - else { - $delete_results['errors']++; - } - } - } - - if ( $delete_results['ok'] > 0 ) { - $msg = __('The selected files were deleted.','cftp_admin'); - echo system_message('success',$msg); - } - if ( $delete_results['errors'] > 0 ) { - $msg = __('Some files could not be deleted.','cftp_admin'); - echo system_message('danger',$msg); - } - break; - case 'edit': - $url = BASE_URI.'files-edit.php?ids='.implode(',', $selected_files); - header("Location: ".$url); - exit; - break; - } - } - else { - $msg = __('Please select at least one file.','cftp_admin'); - echo system_message('danger',$msg); - } - } - /** * Global form action */ @@ -407,8 +409,8 @@
- - + +
@@ -417,8 +419,8 @@ - - + + diff --git a/options.php b/options.php index 30ef95f60..c6be134e0 100644 --- a/options.php +++ b/options.php @@ -306,7 +306,7 @@
- +
@@ -20,14 +22,14 @@ /** * Apply the corresponding action to the selected users. */ - if(isset($_GET['action'])) { + if (isset($_POST['action'])) { /** Continue only if 1 or more users were selected. */ - if(!empty($_GET['batch'])) { - $selected_users = $_GET['batch']; + if (!empty($_POST['batch'])) { + $selected_users = $_POST['batch']; $affected_users = 0; - switch($_GET['action']) { + switch ($_POST['action']) { case 'activate': /** * Changes the value on the "active" column value on the database. @@ -40,9 +42,8 @@ } } - $msg = __('The selected users were marked as active.','cftp_admin'); - echo system_message('success',$msg); - break; + $flash->success(__('The selected users were marked as active.', 'cftp_admin')); + break; case 'deactivate': /** * Reverse of the previous action. Setting the value to 0 means @@ -60,16 +61,14 @@ $affected_users++; } else { - $msg = __('You cannot deactivate your own account.','cftp_admin'); - echo system_message('danger',$msg); + $flash->error(__('You cannot deactivate your own account.', 'cftp_admin')); } } if ($affected_users > 0) { - $msg = __('The selected users were marked as inactive.','cftp_admin'); - echo system_message('success',$msg); + $flash->success(__('The selected users were marked as inactive.', 'cftp_admin')); } - break; + break; case 'delete': foreach ($selected_users as $work_user) { /** @@ -83,22 +82,21 @@ } } else { - $msg = __('You cannot delete your own account.','cftp_admin'); - echo system_message('danger',$msg); + $flash->error(__('You cannot delete your own account.', 'cftp_admin')); } } if ($affected_users > 0) { - $msg = __('The selected users were deleted.','cftp_admin'); - echo system_message('success',$msg); + $flash->success(__('The selected users were deleted.', 'cftp_admin')); } break; } } else { - $msg = __('Please select at least one user.','cftp_admin'); - echo system_message('danger',$msg); + $flash->error(__('Please select at least one user.', 'cftp_admin')); } + + ps_redirect($current_url); } $params = array(); @@ -204,8 +202,8 @@
- - + +
@@ -214,11 +212,11 @@