Security hardening

WEB-INF/lib/ttClientHelper.class.php

@@ -191,6 +191,7 @@ static function insert($fields)
     $last_id = $mdb2->lastInsertID('tt_clients', 'id');
     if (isset($projects) && count($projects) > 0)
       foreach ($projects as $p_id) {
+        $p_id = (int) $p_id;
         $sql = "insert into tt_client_project_binds (client_id, project_id, group_id, org_id) values($last_id, $p_id, $group_id, $org_id)";
         $affected = $mdb2->exec($sql);
         if (is_a($affected, 'PEAR_Error'))
@@ -231,6 +232,7 @@ static function update($fields)
       die($affected->getMessage());
     if (count($projects) > 0)
       foreach ($projects as $p_id) {
+        $p_id = (int) $p_id;
         $sql = "insert into tt_client_project_binds (client_id, project_id, group_id, org_id) values($id, $p_id, $group_id, $org_id)";
         $affected = $mdb2->exec($sql);
         if (is_a($affected, 'PEAR_Error'))

WEB-INF/lib/ttFavReportHelper.class.php

@@ -57,6 +57,7 @@ static function get($id) {
   static function getReport($id) {
     $mdb2 = getConnection();
 
+    $id = (int) $id;
     $sql = "select * from tt_fav_reports where id = $id and status = 1";
     $res = $mdb2->query($sql);
     if (!is_a($res, 'PEAR_Error')) {

WEB-INF/lib/ttOrgImportHelper.class.php

@@ -1025,6 +1025,7 @@ private function insertClient($fields)
 
     $tax = str_replace(',', '.', $tax);
     if ($tax == '') $tax = 0;
+    $tax = (float) $tax;
 
     $sql = "insert into tt_clients (group_id, org_id, name, address, tax, projects, status)".
       " values ($group_id, $org_id, ".$mdb2->quote($name).", ".$mdb2->quote($address).", $tax, ".$mdb2->quote($comma_separated).", ".$mdb2->quote($status).")";
@@ -1037,6 +1038,7 @@ private function insertClient($fields)
 
     if (count($projects) > 0)
       foreach ($projects as $p_id) {
+        $p_id = (int) $p_id;
         $sql = "insert into tt_client_project_binds (client_id, project_id, group_id, org_id) values($last_id, $p_id, $group_id, $org_id)";
         $affected = $mdb2->exec($sql);
         if (is_a($affected, 'PEAR_Error'))

WEB-INF/lib/ttReportHelper.class.php

@@ -21,6 +21,18 @@
 // Class ttReportHelper is used for help with reports.
 class ttReportHelper {
 
+  // makeIdList takes a comma-separated list of ids and returns one that is safe
+  // to interpolate into an "in (...)" clause. An empty result is returned as a
+  // value that matches nothing, so that a list can never widen a query.
+  static function makeIdList($ids) {
+    $id_list = array();
+    foreach (explode(',', $ids) as $id) {
+      $id = trim($id);
+      if (ttValidInteger($id)) $id_list[] = (int) $id;
+    }
+    return $id_list ? join(',', $id_list) : '-1';
+  }
+
   // getWhere prepares a WHERE clause for a report query.
   static function getWhere($options) {
     global $user;
@@ -48,7 +60,7 @@ static function getWhere($options) {
       $dropdown_parts .= ' and l.client_id = '.$user->client_id;
 
     if (isset($options['project_ids']))
-      $dropdown_parts .= ' and l.project_id in ('.$options['project_ids'].')';
+      $dropdown_parts .= ' and l.project_id in ('.ttReportHelper::makeIdList($options['project_ids']).')';
     // if ($options['project_id']) $dropdown_parts .= ' and l.project_id = '.$options['project_id']; // This was here for a single select.
 
     if ($options['task_id']) $dropdown_parts .= ' and l.task_id = '.$options['task_id'];
@@ -137,7 +149,7 @@ static function getWhere($options) {
     }
 
     // Prepare sql query part for user list.
-    $userlist = isset($options['users']) ? $options['users'] : '-1';
+    $userlist = isset($options['users']) ? ttReportHelper::makeIdList($options['users']) : '-1';
     if ($user->can('view_reports') || $user->can('view_all_reports') || $user->isClient())
       $user_list_part = " and l.user_id in ($userlist)";
     else
@@ -186,7 +198,7 @@ static function getExpenseWhere($options) {
       $dropdown_parts .= ' and ei.client_id = '.$user->client_id;
 
     if (isset($options['project_ids']))
-      $dropdown_parts .= ' and ei.project_id in ('.$options['project_ids'].')';
+      $dropdown_parts .= ' and ei.project_id in ('.ttReportHelper::makeIdList($options['project_ids']).')';
     // if ($options['project_id']) $dropdown_parts .= ' and l.project_id = '.$options['project_id']; // This was here for a single select.
 
     if ($options['invoice']==1) $dropdown_parts .= ' and ei.invoice_id is not null';
@@ -253,7 +265,7 @@ static function getExpenseWhere($options) {
     }
 
     // Prepare sql query part for user list.
-    $userlist = isset($options['users']) ? $options['users'] : '-1';
+    $userlist = isset($options['users']) ? ttReportHelper::makeIdList($options['users']) : '-1';
     if ($user->can('view_reports') || $user->can('view_all_reports') || $user->isClient())
       $user_list_part = " and ei.user_id in ($userlist)";
     else
@@ -1128,7 +1140,7 @@ static function assignToTimesheet($timesheet_id, $time_log_ids) {
       // Allow oprations only with pending timesheets.
       if ($timesheet_id) {
         // Assigning a timesheet to records.
-        $inner_join = " inner join tt_timesheets ts on (ts.id = $timesheet_id".
+        $inner_join = " inner join tt_timesheets ts on (ts.id = ".(int)$timesheet_id.
           " and ts.user_id = $user_id and ts.approve_status is null". // Timesheet to assign to is pending.
           // Part below: existing timesheet either not exists or is also pending.
           " and (l.timesheet_id is null or (l.timesheet_id = ts.id and ts.approve_status is null)))";

WEB-INF/lib/ttTaskHelper.class.php

@@ -141,6 +141,7 @@ static function insert($fields)
 
     if (is_array($projects)) {
       foreach ($projects as $p_id) {
+        $p_id = (int) $p_id;
         // Insert task binds into tt_project_task_binds table.
         $sql = "insert into tt_project_task_binds (project_id, task_id, group_id, org_id)".
           " values($p_id, $last_id, $group_id, $org_id)";
@@ -203,6 +204,7 @@ static function update($fields)
       die($affected->getMessage());
     if (count($projects) > 0)
       foreach ($projects as $p_id) {
+        $p_id = (int) $p_id;
         $sql = "insert into tt_project_task_binds (project_id, task_id, group_id, org_id)".
           " values($p_id, $task_id, $group_id, $org_id)";
         $affected = $mdb2->exec($sql);

WEB-INF/lib/ttTimesheetHelper.class.php

@@ -283,6 +283,8 @@ static function getApprover($user_id) {
     global $user;
     $mdb2 = getConnection();
 
+    $user_id = (int) $user_id;
+
     $group_id = $user->getGroup();
     $org_id = $user->org_id;
 

WEB-INF/lib/ttUserHelper.class.php

@@ -105,6 +105,7 @@ static function insert($fields, $hash = true) {
     if (count($projects) > 0) {
       // We have at least one project assigned. Insert corresponding entries in tt_user_project_binds table.
       foreach($projects as $p) {
+        $p['id'] = (int) $p['id'];
         if(!isset($p['rate']))
           $p['rate'] = 0;
         else

charts.php

@@ -119,6 +119,11 @@
   $_SESSION['chart_interval'] = $cl_interval;
 }
 
+if ($cl_fav_report != -1 && !ttFavReportHelper::get($cl_fav_report)) {
+  $cl_fav_report = -1;
+  $_SESSION['fav_report'] = $cl_fav_report;
+}
+
 // Elements of chartForm.
 $chart_form = new Form('chartForm');
 $largeScreenCalendarRowSpan = 1; // Number of rows calendar spans on large screens.

client_add.php

@@ -47,6 +47,7 @@
   if (!ttValidString($cl_name)) $err->add($i18n->get('error.field'), $i18n->get('label.client_name'));
   if (!ttValidString($cl_address, true)) $err->add($i18n->get('error.field'), $i18n->get('label.client_address'));
   if (!ttValidFloat($cl_tax, true)) $err->add($i18n->get('error.field'), $i18n->get('label.tax'));
+  if (!ttGroupHelper::validateCheckboxGroupInput($cl_projects, 'tt_projects')) $err->add($i18n->get('error.field'), $i18n->get('label.projects'));
 
   if ($err->no()) {
     if (!ttClientHelper::getClientByName($cl_name)) {

client_edit.php

@@ -65,6 +65,7 @@
   if (!ttValidString($cl_address, true)) $err->add($i18n->get('error.field'), $i18n->get('label.client_address'));
   if (!ttValidFloat($cl_tax, true)) $err->add($i18n->get('error.field'), $i18n->get('label.tax'));
   if (!ttValidStatus($cl_status)) $err->add($i18n->get('error.field'), $i18n->get('label.status'));
+  if (!ttGroupHelper::validateCheckboxGroupInput($cl_projects, 'tt_projects')) $err->add($i18n->get('error.field'), $i18n->get('label.projects'));
 
   if ($err->no()) {
     if ($request->getParameter('btn_save')) {

report.php

@@ -175,9 +175,9 @@
     // We act on selected records. Are there any?
     foreach($_POST as $key => $val) {
       if ('log_id_' == substr($key, 0, 7))
-        $time_log_ids[] = substr($key, 7);
+        $time_log_ids[] = (int) substr($key, 7);
       if ('item_id_' == substr($key, 0, 8))
-        $expense_item_ids[] = substr($key, 8);
+        $expense_item_ids[] = (int) substr($key, 8);
     }
     if (!$time_log_ids && !$expense_item_ids) $err->Add($i18n->get('error.record')); // There are no selected records.
     // Validation of parameteres ended here.
@@ -194,6 +194,10 @@
   if ($err->no()) {
     if ($request->getParameter('btn_mark_approved')) {
       // User clicked the "Mark approved" button to mark some or all items either approved or not approved.
+      if (!($user->isPluginEnabled('ap') && ($user->can('approve_reports') || $user->can('approve_all_reports')))) {
+        header('Location: access_denied.php');
+        exit();
+      }
 
       // Determine user action.
       $mark_approved = $request->getParameter('mark_approved_action_options') == 1 ? true : false;
@@ -210,6 +214,10 @@
 
     if ($request->getParameter('btn_mark_paid')) {
       // User clicked the "Mark paid" button to mark some or all items either paid or not paid.
+      if (!($user->isPluginEnabled('ps') && $user->can('manage_invoices'))) {
+        header('Location: access_denied.php');
+        exit();
+      }
 
       // Determine user action.
       $mark_paid = $request->getParameter('mark_paid_action_options') == 1 ? true : false;
@@ -226,6 +234,10 @@
 
     if ($request->getParameter('btn_assign_invoice')) {
       // User clicked the Submit button to assign all or some items to a recent invoice.
+      if (!($user->isPluginEnabled('iv') && $user->can('manage_invoices') && $client_id && !$user->isClient())) {
+        header('Location: access_denied.php');
+        exit();
+      }
 
       // Determine invoice id.
       $invoice_id = $request->getParameter('recent_invoice');
@@ -241,6 +253,10 @@
 
     if ($request->getParameter('btn_assign_timesheet')) {
       // User clicked the Submit button to assign all or some items to a timesheet.
+      if (!$user->isPluginEnabled('ts')) {
+        header('Location: access_denied.php');
+        exit();
+      }
 
       // Determine invoice id.
       $timesheet_id = $request->getParameter('timesheet');

task_add.php

@@ -41,6 +41,7 @@
   // Validate user input.
   if (!ttValidString($cl_name, false, MAX_NAME_CHARS)) $err->add($i18n->get('error.field'), $i18n->get('label.thing_name'));
   if (!ttValidString($cl_description, true, MAX_DESCR_CHARS)) $err->add($i18n->get('error.field'), $i18n->get('label.description'));
+  if (!ttGroupHelper::validateCheckboxGroupInput($cl_projects, 'tt_projects')) $err->add($i18n->get('error.field'), $i18n->get('label.projects'));
 
   if ($err->no()) {
     if (!ttTaskHelper::getTaskByName($cl_name)) {

task_edit.php

@@ -57,6 +57,7 @@
   if (!ttValidString($cl_name, false, MAX_NAME_CHARS)) $err->add($i18n->get('error.field'), $i18n->get('label.thing_name'));
   if (!ttValidString($cl_description, true, MAX_DESCR_CHARS)) $err->add($i18n->get('error.field'), $i18n->get('label.description'));
   if (!ttValidStatus($cl_status)) $err->add($i18n->get('error.field'), $i18n->get('label.status'));
+  if (!ttGroupHelper::validateCheckboxGroupInput($cl_projects, 'tt_projects')) $err->add($i18n->get('error.field'), $i18n->get('label.projects'));
 
   if ($err->no()) {
     if ($request->getParameter('btn_save')) {

timesheet_view.php

@@ -27,7 +27,7 @@
 
 if ($request->isPost()) {
   $cl_comment = trim($request->getParameter('comment'));
-  $approver_id = $request->getParameter('approver');
+  $approver_id = (int) $request->getParameter('approver');
 }
 
 $options = ttTimesheetHelper::getReportOptions($timesheet);

user_add.php

@@ -62,6 +62,8 @@
   }
   $cl_rate = $request->getParameter('rate');
   $cl_projects = $request->getParameter('projects');
+  if (!ttGroupHelper::validateCheckboxGroupInput($cl_projects, 'tt_projects'))
+    $err->add($i18n->get('error.field'), $i18n->get('label.projects'));
   if (is_array($cl_projects)) {
     foreach ($cl_projects as $p) {
       if (ttValidFloat($request->getParameter('rate_'.$p), true)) {