From 11bfec0ff88b649a934556e8b4812d65715c7509 Mon Sep 17 00:00:00 2001 From: Nik Okuntseff Date: Mon, 26 Mar 2018 18:45:28 +0000 Subject: [PATCH] Security fix - improved access checks for task edit and deletes. --- WEB-INF/templates/footer.tpl | 2 +- mobile/task_add.php | 1 + mobile/task_delete.php | 7 ++++++- mobile/task_edit.php | 10 +++++++--- mobile/tasks.php | 1 + mobile/time.php | 3 ++- mobile/time_delete.php | 2 +- mobile/time_edit.php | 4 ++-- mobile/user_add.php | 3 ++- mobile/user_delete.php | 4 ++-- mobile/user_edit.php | 4 ++-- mobile/users.php | 3 ++- task_add.php | 1 + task_delete.php | 7 ++++++- task_edit.php | 10 +++++++--- tasks.php | 1 + time.php | 8 +------- time_delete.php | 2 +- time_edit.php | 2 +- tofile.php | 3 ++- topdf.php | 3 ++- user_add.php | 3 ++- user_delete.php | 2 +- user_edit.php | 2 +- users.php | 3 ++- week.php | 1 + 26 files changed, 58 insertions(+), 34 deletions(-) diff --git a/WEB-INF/templates/footer.tpl b/WEB-INF/templates/footer.tpl index 2aa36a25..b9939f90 100644 --- a/WEB-INF/templates/footer.tpl +++ b/WEB-INF/templates/footer.tpl @@ -12,7 +12,7 @@
-
 Anuko Time Tracker 1.17.73.4178 | Copyright © Anuko | +  Anuko Time Tracker 1.17.74.4179 | Copyright © Anuko | {$i18n.footer.credits} | {$i18n.footer.license} | {$i18n.footer.improve} diff --git a/mobile/task_add.php b/mobile/task_add.php index a976ac5c..fd1fb464 100644 --- a/mobile/task_add.php +++ b/mobile/task_add.php @@ -41,6 +41,7 @@ if (MODE_PROJECTS_AND_TASKS != $user->tracking_mode) { header('Location: feature_disabled.php'); exit(); } +// End of access checks. $projects = ttTeamHelper::getActiveProjects($user->team_id); diff --git a/mobile/task_delete.php b/mobile/task_delete.php index 1146a2ac..153f3a2d 100644 --- a/mobile/task_delete.php +++ b/mobile/task_delete.php @@ -39,9 +39,14 @@ if (MODE_PROJECTS_AND_TASKS != $user->tracking_mode) { header('Location: feature_disabled.php'); exit(); } - $cl_task_id = (int)$request->getParameter('id'); $task = ttTaskHelper::get($cl_task_id); +if (!$task) { + header('Location: access_denied.php'); + exit(); +} +// End of access checks. + $task_to_delete = $task['name']; $form = new Form('taskDeleteForm'); diff --git a/mobile/task_edit.php b/mobile/task_edit.php index 346899dc..f1748c7e 100644 --- a/mobile/task_edit.php +++ b/mobile/task_edit.php @@ -40,8 +40,14 @@ if (MODE_PROJECTS_AND_TASKS != $user->tracking_mode) { header('Location: feature_disabled.php'); exit(); } - $cl_task_id = (int)$request->getParameter('id'); +$task = ttTaskHelper::get($cl_task_id); +if (!$task) { + header('Location: access_denied.php'); + exit(); +} +// End of access checks. + $projects = ttTeamHelper::getActiveProjects($user->team_id); if ($request->isPost()) { @@ -50,11 +56,9 @@ if ($request->isPost()) { $cl_status = $request->getParameter('status'); $cl_projects = $request->getParameter('projects'); } else { - $task = ttTaskHelper::get($cl_task_id); $cl_name = $task['name']; $cl_description = $task['description']; $cl_status = $task['status']; - $assigned_projects = ttTaskHelper::getAssignedProjects($cl_task_id); foreach ($assigned_projects as $project_item) $cl_projects[] = $project_item['id']; diff --git a/mobile/tasks.php b/mobile/tasks.php index edb57089..e49498c4 100644 --- a/mobile/tasks.php +++ b/mobile/tasks.php @@ -39,6 +39,7 @@ if (MODE_PROJECTS_AND_TASKS != $user->tracking_mode) { header('Location: feature_disabled.php'); exit(); } +// End of access checks. $smarty->assign('active_tasks', ttTeamHelper::getActiveTasks($user->team_id)); $smarty->assign('inactive_tasks', ttTeamHelper::getInactiveTasks($user->team_id)); diff --git a/mobile/time.php b/mobile/time.php index abde5e01..8c6721d4 100644 --- a/mobile/time.php +++ b/mobile/time.php @@ -34,11 +34,12 @@ import('ttClientHelper'); import('ttTimeHelper'); import('DateAndTime'); -// Access check. +// Access checks. if (!ttAccessAllowed('track_own_time')) { header('Location: access_denied.php'); exit(); } +// End of access checks. // Initialize and store date in session. $cl_date = $request->getParameter('date', @$_SESSION['date']); diff --git a/mobile/time_delete.php b/mobile/time_delete.php index 5d6918c5..399895c7 100644 --- a/mobile/time_delete.php +++ b/mobile/time_delete.php @@ -38,13 +38,13 @@ if (!ttAccessAllowed('track_own_time')) { exit(); } $cl_id = (int)$request->getParameter('id'); -// Get the time record we are deleting. $time_rec = ttTimeHelper::getRecord($cl_id, $user->getActiveUser()); if (!$time_rec || $time_rec['invoice_id']) { // Prohibit deleting not ours or invoiced records. header('Location: access_denied.php'); exit(); } +// End of access checks. // Escape comment for presentation. $time_rec['comment'] = htmlspecialchars($time_rec['comment']); diff --git a/mobile/time_edit.php b/mobile/time_edit.php index eff9b335..a1a56e37 100644 --- a/mobile/time_edit.php +++ b/mobile/time_edit.php @@ -34,19 +34,19 @@ import('ttClientHelper'); import('ttTimeHelper'); import('DateAndTime'); -// Access check. +// Access checks. if (!ttAccessAllowed('track_own_time')) { header('Location: access_denied.php'); exit(); } $cl_id = (int)$request->getParameter('id'); -// Get the time record we are editing. $time_rec = ttTimeHelper::getRecord($cl_id, $user->getActiveUser()); if (!$time_rec || $time_rec['invoice_id']) { // Prohibit editing not ours or invoiced records. header('Location: access_denied.php'); exit(); } +// End of access checks. // Use custom fields plugin if it is enabled. if ($user->isPluginEnabled('cf')) { diff --git a/mobile/user_add.php b/mobile/user_add.php index 838981de..53b9842f 100644 --- a/mobile/user_add.php +++ b/mobile/user_add.php @@ -33,11 +33,12 @@ import('ttUserHelper'); import('form.Table'); import('form.TableColumn'); -// Access check. +// Access checks. if (!ttAccessAllowed('manage_users')) { header('Location: access_denied.php'); exit(); } +// End of access checks. // Use the "limit" plugin if we have one. Ignore include errors. // The "limit" plugin is not required for normal operation of the Time Tracker. diff --git a/mobile/user_delete.php b/mobile/user_delete.php index 4c9aad87..b5c8dae1 100644 --- a/mobile/user_delete.php +++ b/mobile/user_delete.php @@ -30,12 +30,12 @@ require_once('../initialize.php'); import('form.Form'); import('ttUserHelper'); -// Access check. +// Access checks. if (!ttAccessAllowed('manage_users')) { header('Location: access_denied.php'); exit(); } -$user_id = (int) $request->getParameter('id'); +$user_id = (int)$request->getParameter('id'); $user_details = $user->getUser($user_id); if (!$user_details) { header('Location: access_denied.php'); diff --git a/mobile/user_edit.php b/mobile/user_edit.php index 3baaf1e9..f82d9314 100644 --- a/mobile/user_edit.php +++ b/mobile/user_edit.php @@ -34,12 +34,12 @@ import('ttUserHelper'); import('form.Table'); import('form.TableColumn'); -// Access check. +// Access checks. if (!ttAccessAllowed('manage_users')) { header('Location: access_denied.php'); exit(); } -$user_id = (int) $request->getParameter('id'); +$user_id = (int)$request->getParameter('id'); $user_details = $user->getUser($user_id); if (!$user_details) { header('Location: access_denied.php'); diff --git a/mobile/users.php b/mobile/users.php index 67654376..18ad2e5f 100644 --- a/mobile/users.php +++ b/mobile/users.php @@ -31,11 +31,12 @@ import('form.Form'); import('ttTeamHelper'); import('ttTimeHelper'); -// Access check. +// Access checks. if (!(ttAccessAllowed('view_users') || ttAccessAllowed('manage_users'))) { header('Location: access_denied.php'); exit(); } +// End of access checks. // Get users. $active_users = ttTeamHelper::getActiveUsers(array('getAllFields'=>true)); diff --git a/task_add.php b/task_add.php index a5149e84..eaaba5e8 100644 --- a/task_add.php +++ b/task_add.php @@ -41,6 +41,7 @@ if (MODE_PROJECTS_AND_TASKS != $user->tracking_mode) { header('Location: feature_disabled.php'); exit(); } +// End of access checks. $projects = ttTeamHelper::getActiveProjects($user->team_id); diff --git a/task_delete.php b/task_delete.php index 0e9f40cf..d8e14399 100644 --- a/task_delete.php +++ b/task_delete.php @@ -39,9 +39,14 @@ if (MODE_PROJECTS_AND_TASKS != $user->tracking_mode) { header('Location: feature_disabled.php'); exit(); } - $cl_task_id = (int)$request->getParameter('id'); $task = ttTaskHelper::get($cl_task_id); +if (!$task) { + header('Location: access_denied.php'); + exit(); +} +// End of access checks. + $task_to_delete = $task['name']; $form = new Form('taskDeleteForm'); diff --git a/task_edit.php b/task_edit.php index bba89beb..324f1dbb 100644 --- a/task_edit.php +++ b/task_edit.php @@ -40,8 +40,14 @@ if (MODE_PROJECTS_AND_TASKS != $user->tracking_mode) { header('Location: feature_disabled.php'); exit(); } - $cl_task_id = (int)$request->getParameter('id'); +$task = ttTaskHelper::get($cl_task_id); +if (!$task) { + header('Location: access_denied.php'); + exit(); +} +// End of access checks. + $projects = ttTeamHelper::getActiveProjects($user->team_id); if ($request->isPost()) { @@ -50,11 +56,9 @@ if ($request->isPost()) { $cl_status = $request->getParameter('status'); $cl_projects = $request->getParameter('projects'); } else { - $task = ttTaskHelper::get($cl_task_id); $cl_name = $task['name']; $cl_description = $task['description']; $cl_status = $task['status']; - $assigned_projects = ttTaskHelper::getAssignedProjects($cl_task_id); foreach ($assigned_projects as $project_item) $cl_projects[] = $project_item['id']; diff --git a/tasks.php b/tasks.php index 5505e6dd..2d310d01 100644 --- a/tasks.php +++ b/tasks.php @@ -39,6 +39,7 @@ if (MODE_PROJECTS_AND_TASKS != $user->tracking_mode) { header('Location: feature_disabled.php'); exit(); } +// End of access checks. $smarty->assign('active_tasks', ttTeamHelper::getActiveTasks($user->team_id)); $smarty->assign('inactive_tasks', ttTeamHelper::getInactiveTasks($user->team_id)); diff --git a/time.php b/time.php index 98a383fd..d75854c8 100644 --- a/time.php +++ b/time.php @@ -34,13 +34,6 @@ import('ttClientHelper'); import('ttTimeHelper'); import('DateAndTime'); -// This is a now removed check whether user browser supports cookies. -// if (!isset($_COOKIE['tt_PHPSESSID'])) { - // This test gives a false-positive if user goes directly to this page - // as from a desktop shortcut (on first request only). - // die ("Your browser's cookie functionality is turned off. Please turn it on."); -// } - // Access checks. if (!(ttAccessAllowed('track_own_time') || ttAccessAllowed('track_time'))) { header('Location: access_denied.php'); @@ -54,6 +47,7 @@ if (!$user->behalf_id && !$user->can('track_own_time') && !$user->adjustBehalfId header('Location: access_denied.php'); // Trying as self, but no right for self, and noone to work on behalf. exit(); } +// End of access checks. // Initialize and store date in session. $cl_date = $request->getParameter('date', @$_SESSION['date']); diff --git a/time_delete.php b/time_delete.php index 060311e8..5d721ab1 100644 --- a/time_delete.php +++ b/time_delete.php @@ -38,13 +38,13 @@ if (!(ttAccessAllowed('track_own_time') || ttAccessAllowed('track_time'))) { exit(); } $cl_id = (int)$request->getParameter('id'); -// Get the time record we are deleting. $time_rec = ttTimeHelper::getRecord($cl_id, $user->getActiveUser()); if (!$time_rec || $time_rec['invoice_id']) { // Prohibit deleting not ours or invoiced records. header('Location: access_denied.php'); exit(); } +// End of access checks. // Escape comment for presentation. $time_rec['comment'] = htmlspecialchars($time_rec['comment']); diff --git a/time_edit.php b/time_edit.php index 05db97c0..507f28ab 100644 --- a/time_edit.php +++ b/time_edit.php @@ -40,13 +40,13 @@ if (!(ttAccessAllowed('track_own_time') || ttAccessAllowed('track_time'))) { exit(); } $cl_id = (int)$request->getParameter('id'); -// Get the time record we are editing. $time_rec = ttTimeHelper::getRecord($cl_id, $user->getActiveUser()); if (!$time_rec || $time_rec['invoice_id']) { // Prohibit editing not ours or invoiced records. header('Location: access_denied.php'); exit(); } +// End of access checks. // Use custom fields plugin if it is enabled. if ($user->isPluginEnabled('cf')) { diff --git a/tofile.php b/tofile.php index abd1f278..e7b9ed96 100644 --- a/tofile.php +++ b/tofile.php @@ -31,11 +31,12 @@ import('form.Form'); import('form.ActionForm'); import('ttReportHelper'); -// Access check. +// Access checks. if (!(ttAccessAllowed('view_own_reports') || ttAccessAllowed('view_reports'))) { header('Location: access_denied.php'); exit(); } +// End of access checks. // Use custom fields plugin if it is enabled. if ($user->isPluginEnabled('cf')) { diff --git a/topdf.php b/topdf.php index 3177d155..475522ef 100644 --- a/topdf.php +++ b/topdf.php @@ -35,11 +35,12 @@ import('form.Form'); import('form.ActionForm'); import('ttReportHelper'); -// Access check. +// Access checks. if (!(ttAccessAllowed('view_own_reports') || ttAccessAllowed('view_reports'))) { header('Location: access_denied.php'); exit(); } +// End of access checks. // Check whether TCPDF library is available. if (!file_exists('WEB-INF/lib/tcpdf/')) diff --git a/user_add.php b/user_add.php index 91286537..b235cbd1 100644 --- a/user_add.php +++ b/user_add.php @@ -34,11 +34,12 @@ import('form.Table'); import('form.TableColumn'); import('ttRoleHelper'); -// Access check. +// Access checks. if (!ttAccessAllowed('manage_users')) { header('Location: access_denied.php'); exit(); } +// End of access checks. // Use the "limit" plugin if we have one. Ignore include errors. // The "limit" plugin is not required for normal operation of the Time Tracker. diff --git a/user_delete.php b/user_delete.php index d06463b8..647f2172 100644 --- a/user_delete.php +++ b/user_delete.php @@ -35,7 +35,7 @@ if (!ttAccessAllowed('manage_users')) { header('Location: access_denied.php'); exit(); } -$user_id = (int) $request->getParameter('id'); +$user_id = (int)$request->getParameter('id'); $user_details = $user->getUser($user_id); if (!$user_details) { header('Location: access_denied.php'); diff --git a/user_edit.php b/user_edit.php index 8f588db9..95ed29e7 100644 --- a/user_edit.php +++ b/user_edit.php @@ -40,7 +40,7 @@ if (!ttAccessAllowed('manage_users')) { header('Location: access_denied.php'); exit(); } -$user_id = (int) $request->getParameter('id'); +$user_id = (int)$request->getParameter('id'); $user_details = $user->getUser($user_id); if (!$user_details) { header('Location: access_denied.php'); diff --git a/users.php b/users.php index 8787844b..af538902 100644 --- a/users.php +++ b/users.php @@ -32,11 +32,12 @@ import('ttTeamHelper'); import('ttTimeHelper'); import('ttRoleHelper'); -// Access check. +// Access checks. if (!(ttAccessAllowed('view_users') || ttAccessAllowed('manage_users'))) { header('Location: access_denied.php'); exit(); } +// End of access checks. // Prepare a list of active users. if ($user->can('view_users')) diff --git a/week.php b/week.php index 741a2bb4..1d72ce40 100644 --- a/week.php +++ b/week.php @@ -55,6 +55,7 @@ if (!$user->behalf_id && !$user->can('track_own_time') && !$user->adjustBehalfId header('Location: access_denied.php'); // Trying as self, but no right for self, and noone to work on behalf. exit(); } +// End of access checks. // Initialize and store date in session. $cl_date = $request->getParameter('date', @$_SESSION['date']); -- 2.20.1