From e77be7eea69df5d52e19f9f25b5b89a0e66a5b8e Mon Sep 17 00:00:00 2001 From: Nik Okuntseff Date: Sun, 11 Apr 2021 21:57:58 +0000 Subject: [PATCH 001/408] An attempt to mitigate cross site request forgery vulnerability. --- WEB-INF/config.php.dist | 5 +++++ WEB-INF/lib/common.lib.php | 39 ++++++++++++++++++++++++++++++++++++++ initialize.php | 2 +- 3 files changed, 45 insertions(+), 1 deletion(-) diff --git a/WEB-INF/config.php.dist b/WEB-INF/config.php.dist index 7b6eabbd2..1b7b32b0a 100644 --- a/WEB-INF/config.php.dist +++ b/WEB-INF/config.php.dist @@ -163,6 +163,11 @@ define('AUTH_MODULE', 'db'); // define('DEBUG', false); // Note: enabling DEBUG breaks redirects as debug output is printed before setting redirect header. Do not enable on production systems. +// HTTP_TARGET - defines http target for cross site request forgery protection. +// It can be used when you access the application via a proxy. +// define('HTTP_TARGET', 'localhost'); + + // Group managers can set monthly work hour quota for years between the following values. // define('MONTHLY_QUOTA_YEAR_START', 2010); // If nothing is specified, it falls back to 2015. // define('MONTHLY_QUOTA_YEAR_END', 2025); // If nothing is specified, it falls back to 2030. diff --git a/WEB-INF/lib/common.lib.php b/WEB-INF/lib/common.lib.php index 389b29c94..52589c6e0 100644 --- a/WEB-INF/lib/common.lib.php +++ b/WEB-INF/lib/common.lib.php @@ -363,6 +363,10 @@ function ttAccessAllowed($required_right) exit(); } + // Protection against cross site request forgery. + if (!ttMitigateCSRF()) + return false; + // Check IP restriction, if set. if ($user->allow_ip && !$user->can('override_allow_ip')) { $access_allowed = false; @@ -388,6 +392,41 @@ function ttAccessAllowed($required_right) return false; } +// ttMitigateCSRF verifies request headers in an attempt to block cross site request forgery. +function ttMitigateCSRF() { + // No need to do anything for get requests. + global $request; + if ($request->isGet()) + return true; + + $origin = $_SERVER['HTTP_ORIGIN']; + if ($origin) { + $pos = strpos($origin, '//'); + $origin = substr($origin, $pos+2); // Strip protocol. + } + if (!$origin) { + // Try using referer. + $origin = $_SERVER['HTTP_REFERER']; + if ($origin) { + $pos = strpos($origin, '//'); + $origin = substr($origin, $pos+2); // Strip protocol. + $pos = strpos($origin, '/'); + $origin = substr($origin, 0, $pos); // Leave host only. + } + } + error_log("origin: ".$origin); + $target = defined('HTTP_TARGET') ? HTTP_TARGET : $_SERVER['HTTP_HOST']; + error_log("target: ".$target); + if (strcmp($origin, $target)) { + error_log("Potential cross site request forgery. Origin: '$origin' does not match target: '$target'."); + return false; // Origin and target do not match, + } + + // TODO: review and improve this function for custom ports. + return true; +} + + // ttStartsWith functions checks if a string starts with a given substring. function ttStartsWith($string, $startString) { diff --git a/initialize.php b/initialize.php index cbdfb3cc1..aadc0bb07 100644 --- a/initialize.php +++ b/initialize.php @@ -13,7 +13,7 @@ ini_set('display_errors', 'Off'); // require_once('init_auth.php'); -define("APP_VERSION", "1.19.26.5430"); +define("APP_VERSION", "1.19.27.5431"); define("APP_DIR", dirname(__FILE__)); define("LIBRARY_DIR", APP_DIR."/WEB-INF/lib"); define("TEMPLATE_DIR", APP_DIR."/WEB-INF/templates"); From e3f8222ee308322942bcebcd86b78ecf19382563 Mon Sep 17 00:00:00 2001 From: Nik Okuntseff Date: Mon, 12 Apr 2021 17:14:30 +0000 Subject: [PATCH 002/408] Tested CSRF fix for custom ports - it's working, also removed unnecessary logging. --- WEB-INF/config.php.dist | 2 +- WEB-INF/lib/common.lib.php | 5 +---- initialize.php | 2 +- 3 files changed, 3 insertions(+), 6 deletions(-) diff --git a/WEB-INF/config.php.dist b/WEB-INF/config.php.dist index 1b7b32b0a..3787cbc94 100644 --- a/WEB-INF/config.php.dist +++ b/WEB-INF/config.php.dist @@ -165,7 +165,7 @@ define('AUTH_MODULE', 'db'); // HTTP_TARGET - defines http target for cross site request forgery protection. // It can be used when you access the application via a proxy. -// define('HTTP_TARGET', 'localhost'); +// define('HTTP_TARGET', 'localhost:8080'); // Group managers can set monthly work hour quota for years between the following values. diff --git a/WEB-INF/lib/common.lib.php b/WEB-INF/lib/common.lib.php index 52589c6e0..d5b43f57f 100644 --- a/WEB-INF/lib/common.lib.php +++ b/WEB-INF/lib/common.lib.php @@ -414,15 +414,12 @@ function ttMitigateCSRF() { $origin = substr($origin, 0, $pos); // Leave host only. } } - error_log("origin: ".$origin); $target = defined('HTTP_TARGET') ? HTTP_TARGET : $_SERVER['HTTP_HOST']; - error_log("target: ".$target); if (strcmp($origin, $target)) { error_log("Potential cross site request forgery. Origin: '$origin' does not match target: '$target'."); - return false; // Origin and target do not match, + return false; // Origin and target do not match. } - // TODO: review and improve this function for custom ports. return true; } diff --git a/initialize.php b/initialize.php index aadc0bb07..e8588857f 100644 --- a/initialize.php +++ b/initialize.php @@ -13,7 +13,7 @@ ini_set('display_errors', 'Off'); // require_once('init_auth.php'); -define("APP_VERSION", "1.19.27.5431"); +define("APP_VERSION", "1.19.27.5432"); define("APP_DIR", dirname(__FILE__)); define("LIBRARY_DIR", APP_DIR."/WEB-INF/lib"); define("TEMPLATE_DIR", APP_DIR."/WEB-INF/templates"); From ee6312e1a278cfbb240ae1419d6c63eeff0913f6 Mon Sep 17 00:00:00 2001 From: Nik Okuntseff Date: Tue, 13 Apr 2021 22:12:35 +0000 Subject: [PATCH 003/408] Added options for secure ldap authentication. --- WEB-INF/config.php.dist | 4 ++++ WEB-INF/lib/auth/Auth_ldap.class.php | 10 ++++++++++ initialize.php | 2 +- 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/WEB-INF/config.php.dist b/WEB-INF/config.php.dist index 3787cbc94..bbf647cec 100644 --- a/WEB-INF/config.php.dist +++ b/WEB-INF/config.php.dist @@ -144,6 +144,8 @@ define('AUTH_MODULE', 'db'); // 'base_dn' => 'ou=People,dc=example,dc=com', // Path of user's base distinguished name in LDAP catalog. // 'user_login_attribute' => 'uid', // LDAP attribute used for login. // 'default_domain' => 'example.com', // Default domain. +// 'tls_cacertdir' => null, // Path to a directory containing CA certificates for secure ldap. +// 'tls_cacertfile' => null, // CA certificate file name for secure ldap. // 'member_of' => array()); // List of groups, membership in which is required for user to be authenticated. @@ -154,6 +156,8 @@ define('AUTH_MODULE', 'db'); // 'type' => 'ad', // Type of server. // 'base_dn' => 'DC=example,DC=com', // Base distinguished name in LDAP catalog. // 'default_domain' => 'example.com', // Default domain. +// 'tls_cacertdir' => null, // Path to a directory containing CA certificates for secure ldap. +// 'tls_cacertfile' => null, // CA certificate file name for secure ldap. // 'member_of' => array()); // List of groups, membership in which is required for user to be authenticated. // Leave it empty if membership is not necessary. Otherwise list CN parts only. // For example: diff --git a/WEB-INF/lib/auth/Auth_ldap.class.php b/WEB-INF/lib/auth/Auth_ldap.class.php index 7f3495737..1ccbb0371 100644 --- a/WEB-INF/lib/auth/Auth_ldap.class.php +++ b/WEB-INF/lib/auth/Auth_ldap.class.php @@ -105,6 +105,16 @@ function authenticate($login, $password) if (isTrue('DEBUG')) { ldap_set_option($lc, LDAP_OPT_DEBUG_LEVEL, 7); } + // Additional options for secure ldap. + // This insert is based on https://www.anuko.com/forum/viewtopic.php?f=4&t=2091 + // I can't test it at the moment. If things break please let us know! + if (isset($this->params['tls_cacertdir'])) { + ldap_set_option(null, LDAP_OPT_X_TLS_CACERTDIR, $this->params['tls_cacertdir']); + } + if (isset($this->params['tls_cacertfile'])) { + ldap_set_option(null, LDAP_OPT_X_TLS_CACERTFILE, $this->params['tls_cacertfile']); + } + // End of addiitional options for secure ldap. // We need to handle Windows AD and OpenLDAP differently. if ($this->params['type'] == 'ad') { diff --git a/initialize.php b/initialize.php index e8588857f..a739b939e 100644 --- a/initialize.php +++ b/initialize.php @@ -13,7 +13,7 @@ ini_set('display_errors', 'Off'); // require_once('init_auth.php'); -define("APP_VERSION", "1.19.27.5432"); +define("APP_VERSION", "1.19.28.5433"); define("APP_DIR", dirname(__FILE__)); define("LIBRARY_DIR", APP_DIR."/WEB-INF/lib"); define("TEMPLATE_DIR", APP_DIR."/WEB-INF/templates"); From 4fdaa18d343d0459391e62b40971843fbd0e5924 Mon Sep 17 00:00:00 2001 From: Nik Okuntseff Date: Mon, 19 Apr 2021 21:26:18 +0000 Subject: [PATCH 004/408] Resuming work on php8 warnings. --- WEB-INF/lib/ttTimeHelper.class.php | 2 ++ WEB-INF/templates/header2.tpl | 2 +- WEB-INF/templates/time2.tpl | 2 +- WEB-INF/templates/time_script.tpl | 16 +++++++++------- initialize.php | 2 +- time.php | 1 + 6 files changed, 15 insertions(+), 10 deletions(-) diff --git a/WEB-INF/lib/ttTimeHelper.class.php b/WEB-INF/lib/ttTimeHelper.class.php index f0b1ea100..af2a436a5 100644 --- a/WEB-INF/lib/ttTimeHelper.class.php +++ b/WEB-INF/lib/ttTimeHelper.class.php @@ -895,6 +895,8 @@ static function getRecords($date, $includeFiles = false) { $time_fields = ", ".join(', ', $time_fields_array); } + $filePart = ''; + $fileJoin = ''; if ($includeFiles) { $filePart = ', if(Sub1.entity_id is null, 0, 1) as has_files'; $fileJoin = " left join (select distinct entity_id from tt_files". diff --git a/WEB-INF/templates/header2.tpl b/WEB-INF/templates/header2.tpl index 6bc5f65fe..74ccb1adc 100644 --- a/WEB-INF/templates/header2.tpl +++ b/WEB-INF/templates/header2.tpl @@ -6,7 +6,7 @@ -{if $i18n.language.rtl} +{if (isset($i18n.language.rtl) && $i18n.language.rtl)} {/if} {if $user->getCustomCss()} diff --git a/WEB-INF/templates/time2.tpl b/WEB-INF/templates/time2.tpl index 487bff7cf..4a26f9bef 100644 --- a/WEB-INF/templates/time2.tpl +++ b/WEB-INF/templates/time2.tpl @@ -93,7 +93,7 @@ License: See license.txt *}
{/if} -{if $template_dropdown} +{if (isset($template_dropdown) && $template_dropdown)} diff --git a/WEB-INF/templates/time_script.tpl b/WEB-INF/templates/time_script.tpl index 5834ba3cc..67c5062cc 100644 --- a/WEB-INF/templates/time_script.tpl +++ b/WEB-INF/templates/time_script.tpl @@ -46,23 +46,25 @@ var task_names = new Array(); // Prepare an array of template ids for projects. var template_ids = new Array(); -{if $bind_templates_with_projects} +{if (isset($bind_templates_with_projects) && $bind_templates_with_projects)} {foreach $project_list as $project} template_ids[{$project.id}] = "{$project.templates}"; {/foreach} {/if} // Prepare an array of template names. var template_names = new Array(); -{if $bind_templates_with_projects} +{if (isset($bind_templates_with_projects) && $bind_templates_with_projects) && isset($template_list)} {foreach $template_list as $template} template_names[{$template.id}] = "{$template.name|escape:'javascript'}"; {/foreach} {/if} // Prepare an array of template bodies. var template_bodies = new Array(); -{foreach $template_list as $template} - template_bodies[{$template.id}] = "{$template.content|escape:'javascript'}"; -{/foreach} +{if isset($template_list)} + {foreach $template_list as $template} + template_bodies[{$template.id}] = "{$template.content|escape:'javascript'}"; + {/foreach} +{/if} // The fillNote function populates the Note field with a selected template body. function fillNote(id) { @@ -189,7 +191,7 @@ function fillTaskDropdown(id) { // The fillTemplateDropdown function populates the template combo box with // templates associated with a selected project (project id is passed here as id). function fillTemplateDropdown(id) { -{if !$bind_templates_with_projects} +{if (!isset($bind_templates_with_projects) || !$bind_templates_with_projects)} return; // Do nothing if we are not binding templates with projects, {/if} @@ -234,7 +236,7 @@ function fillTemplateDropdown(id) { // The prepopulateNote function populates the note field with first found template body in Template dropdown. function prepopulateNote() { - {if !$prepopulate_note} + {if (!isset($prepopulate_note) || !$prepopulate_note)} return; {/if} var dropdown = document.getElementById("template"); diff --git a/initialize.php b/initialize.php index a739b939e..d34aa56d5 100644 --- a/initialize.php +++ b/initialize.php @@ -13,7 +13,7 @@ ini_set('display_errors', 'Off'); // require_once('init_auth.php'); -define("APP_VERSION", "1.19.28.5433"); +define("APP_VERSION", "1.19.28.5434"); define("APP_DIR", dirname(__FILE__)); define("LIBRARY_DIR", APP_DIR."/WEB-INF/lib"); define("TEMPLATE_DIR", APP_DIR."/WEB-INF/templates"); diff --git a/time.php b/time.php index 66cf4da4b..90d54c0cf 100644 --- a/time.php +++ b/time.php @@ -225,6 +225,7 @@ $largeScreenCalendarRowSpan += 2; // Client dropdown. + $client_list = array(); if ($showClient) { $active_clients = ttGroupHelper::getActiveClients(true); // We need an array of assigned project ids to do some trimming. From 1ce02deaaa3f2c66162a55fe0b62acb1dd94739a Mon Sep 17 00:00:00 2001 From: Nik Okuntseff Date: Mon, 19 Apr 2021 22:11:54 +0000 Subject: [PATCH 005/408] Addressed a few more php8 warnings. --- WEB-INF/lib/ttTimeHelper.class.php | 4 ++-- WEB-INF/templates/header2.tpl | 8 ++++---- WEB-INF/templates/time_edit2.tpl | 2 +- initialize.php | 2 +- time_edit.php | 2 ++ 5 files changed, 10 insertions(+), 8 deletions(-) diff --git a/WEB-INF/lib/ttTimeHelper.class.php b/WEB-INF/lib/ttTimeHelper.class.php index af2a436a5..78e7a2cf5 100644 --- a/WEB-INF/lib/ttTimeHelper.class.php +++ b/WEB-INF/lib/ttTimeHelper.class.php @@ -424,10 +424,10 @@ static function insert($fields) $client = $fields['client']; $project = $fields['project']; $task = $fields['task']; - $invoice = $fields['invoice']; + $invoice = isset($fields['invoice']) ? $fields['invoice'] : null; $note = $fields['note']; $billable = $fields['billable']; - $paid = $fields['paid']; + $paid = isset($fields['paid']) ? $fields['paid'] : null; $start = ttTimeHelper::to24HourFormat($start); if ($finish) { diff --git a/WEB-INF/templates/header2.tpl b/WEB-INF/templates/header2.tpl index 74ccb1adc..65af554ee 100644 --- a/WEB-INF/templates/header2.tpl +++ b/WEB-INF/templates/header2.tpl @@ -21,7 +21,7 @@ - +