From f175aaae35dd9e4afa18b9f06b3dcaf448b52c7a Mon Sep 17 00:00:00 2001 From: Michiel Dethmers Date: Sun, 3 Mar 2024 15:16:59 +0000 Subject: [PATCH 1/6] add notification by email when an admin logs in from a new IP address. --- public_html/lists/admin/defaultconfig.php | 8 ++++- public_html/lists/admin/index.php | 27 ++++++++++++-- public_html/lists/admin/lib.php | 36 +++++++++++++++++++ .../admin/phpListAdminAuthentication.php | 13 ++++--- public_html/lists/admin/structure.php | 10 +++++- public_html/lists/admin/upgrade.php | 5 +++ 6 files changed, 90 insertions(+), 9 deletions(-) diff --git a/public_html/lists/admin/defaultconfig.php b/public_html/lists/admin/defaultconfig.php index 548cedbe5..3a6244cdc 100644 --- a/public_html/lists/admin/defaultconfig.php +++ b/public_html/lists/admin/defaultconfig.php @@ -125,7 +125,13 @@ 'type' => 'text', 'category' => 'security', ), - + 'notify_admin_login' => array( + 'value' => 1, + 'description' => s('Notify admin on login from new location'), + 'type' => 'boolean', + 'category' => 'security', + 'allowempty' => true, + ), // admin addresses are other people who receive copies of subscriptions 'admin_addresses' => array( 'value' => '', diff --git a/public_html/lists/admin/index.php b/public_html/lists/admin/index.php index fdcb7182f..add8da570 100644 --- a/public_html/lists/admin/index.php +++ b/public_html/lists/admin/index.php @@ -312,6 +312,10 @@ function mb_strtolower($string) $msg = $loginresult[1]; } else { session_regenerate_id(); + + # invalidate other active sessions + Sql_Query(sprintf('update %s set active = 0 where adminid = %d and active != 0',$GLOBALS['tables']['admin_login'],$loginresult[0])); + $_SESSION['adminloggedin'] = $remoteAddr; $_SESSION['logindetails'] = array( 'adminname' => $_REQUEST['login'], @@ -325,6 +329,19 @@ function mb_strtolower($string) if (!empty($_POST['page'])) { $page = preg_replace('/\W+/', '', $_POST['page']); } + + # check if this is a new IP address + $knownIP = Sql_Fetch_Row_Query(sprintf('select * from %s where remote_ip4 = "%s"',$GLOBALS['tables']['admin_login'],$remoteAddr)); + if (empty($knownIP[0])) { + notifyNewIPLogin($loginresult[0]); + } + Sql_Query(sprintf('insert into %s (moment,adminid,remote_ip4,remote_ip6,sessionid,active) + values(%d,%d,"%s","%s","%s",1)', + $GLOBALS['tables']['admin_login'],time(),$loginresult[0],$remoteAddr,"",session_id())); + + + + } //If passwords are encrypted and a password recovery request was made, send mail to the admin of the given email address. } elseif (isset($_REQUEST['forgotpassword'])) { @@ -373,14 +390,20 @@ function mb_strtolower($string) $_SESSION['logindetails'] = ''; $page = 'login'; } elseif ($_SESSION['adminloggedin'] && $_SESSION['logindetails']) { + $active = Sql_Fetch_Row_Query(sprintf('select active from %s where adminid = %d and (remote_ip4 = "%s" or remote_ip6 = "%s") and sessionid = "%s"', + $GLOBALS['tables']['admin_login'],$_SESSION['logindetails']['id'],$remoteAddr,"",session_id())); $validate = $GLOBALS['admin_auth']->validateAccount($_SESSION['logindetails']['id']); - if (!$validate[0]) { + if (empty($active[0]) || !$validate[0]) { logEvent(sprintf($GLOBALS['I18N']->get('invalidated login from %s for %s (error %s)'), $remoteAddr, $_SESSION['logindetails']['adminname'], $validate[1])); $_SESSION['adminloggedin'] = ''; $_SESSION['logindetails'] = ''; $page = 'login'; - $msg = $validate[1]; + if (empty($active[0])) { + $msg = s('Your session was invalidated by a new session in a different browser'); + } else { + $msg = $validate[1]; + } } } else { $page = 'login'; diff --git a/public_html/lists/admin/lib.php b/public_html/lists/admin/lib.php index 8862710ef..e516b5bb0 100644 --- a/public_html/lists/admin/lib.php +++ b/public_html/lists/admin/lib.php @@ -2486,3 +2486,39 @@ function getClientIP() return $the_ip; } + + +function notifyNewIPLogin($adminId) { + + $enabled = getConfig('notify_admin_login'); + if (empty($enabled)) { + return; + } + $msg = s(' + +-------------------------------------------------------------------------------- + +We noticed a login to your phpList installation at https://%s +from a new location. If this was you, you can delete this message. If you do not recognise +this, please login to your phpList installation and change your password. + +-------------------------------------------------------------------------------- ', +$GLOBALS['config']['website']); + + $admin_mail = $GLOBALS['admin_auth']->adminEmail($_SESSION['logindetails']['id']); + sendMail($admin_mail, $GLOBALS['installation_name'].' '.s('login from new location'), $msg, system_messageheaders($admin_mail)); + + $ok = sendAdminCopy(s('login from new location'), "\n".$msg); + if ($ok === 0) { + $main_admin_mail = getConfig('admin_address'); + logEvent(sprintf('Error sending login notification to %s', $admin_mail)); + + $msg = s(' +--------------------- + phpList tried sending the below message to '.$admin_mail.' + but this failed. +------------------').PHP_EOL.PHP_EOL.$msg; + sendMail($main_admin_mail, $GLOBALS['installation_name'].' '.s('login from new location'), $msg, system_messageheaders($admin_mail)); + } + +} \ No newline at end of file diff --git a/public_html/lists/admin/phpListAdminAuthentication.php b/public_html/lists/admin/phpListAdminAuthentication.php index d2b26e765..7018d6ea6 100644 --- a/public_html/lists/admin/phpListAdminAuthentication.php +++ b/public_html/lists/admin/phpListAdminAuthentication.php @@ -37,12 +37,15 @@ public function validateLogin($login, $password) $passwordDB = $admindata['password']; //Password encryption verification. if (strlen($passwordDB) < $GLOBALS['hash_length']) { // Passwords are encrypted but the actual is not. + return array(0, s('incorrect password')); + + // the below is actually insecure, it allows resetting the password without approval, so remove //Encrypt the actual DB password before performing the validation below. - $encryptedPassDB = hash(HASH_ALGO, $passwordDB); - $query = sprintf('update %s set password = "%s" where loginname = "%s"', $GLOBALS['tables']['admin'], - $encryptedPassDB, sql_escape($login)); - $passwordDB = $encryptedPassDB; - $req = Sql_Query($query); + // $encryptedPassDB = hash(HASH_ALGO, $passwordDB); + // $query = sprintf('update %s set password = "%s" where loginname = "%s"', $GLOBALS['tables']['admin'], + // $encryptedPassDB, sql_escape($login)); + // $passwordDB = $encryptedPassDB; + // $req = Sql_Query($query); } if ($admindata['disabled']) { diff --git a/public_html/lists/admin/structure.php b/public_html/lists/admin/structure.php index 703f3e9c1..a4b2bdd64 100644 --- a/public_html/lists/admin/structure.php +++ b/public_html/lists/admin/structure.php @@ -321,7 +321,15 @@ 'admin' => array('integer', "Admin's Id"), 'key_value' => array('varchar (32) not null', 'Key'), ), - /* + 'admin_login' => array( + 'id' => array('integer not null primary key auto_increment', 'Id'), + 'moment' => array('bigint', 'epoch when it happened'), + 'adminid' => array('integer', "Admin Id"), + 'remote_ip4' => array('varchar (32) not null', 'IPv4 address'), + 'remote_ip6' => array('varchar (50) not null', 'IPv6 address'), + 'sessionid' => array('varchar (50) not null', 'Session ID'), + 'active' => array('tinyint', 'is this login active'), + ), /* * obsolete tables "task" => array( "id" => array("integer not null primary key auto_increment","ID"), diff --git a/public_html/lists/admin/upgrade.php b/public_html/lists/admin/upgrade.php index b10e9697b..106ad2ff7 100644 --- a/public_html/lists/admin/upgrade.php +++ b/public_html/lists/admin/upgrade.php @@ -468,6 +468,11 @@ function output($message) Sql_Query("alter table {$GLOBALS['tables']['admin']} modify modifiedby varchar(66) default ''"); } + if (!Sql_Table_exists($GLOBALS['tables']['admin_login'])) { + cl_output(s('Creating new table "admin_login"')); + createTable('admin_login'); + } + //# longblobs are better at mixing character encoding. We don't know the encoding of anything we may want to store in cache //# before converting, it's quickest to clear the cache clearPageCache(); From 4be526014ab60ba8428fdb00a4d0638c19d63d33 Mon Sep 17 00:00:00 2001 From: Michiel Dethmers Date: Sat, 9 Mar 2024 09:23:22 +0000 Subject: [PATCH 2/6] check IP per admin --- public_html/lists/admin/index.php | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/public_html/lists/admin/index.php b/public_html/lists/admin/index.php index add8da570..d04ed3876 100644 --- a/public_html/lists/admin/index.php +++ b/public_html/lists/admin/index.php @@ -331,17 +331,13 @@ function mb_strtolower($string) } # check if this is a new IP address - $knownIP = Sql_Fetch_Row_Query(sprintf('select * from %s where remote_ip4 = "%s"',$GLOBALS['tables']['admin_login'],$remoteAddr)); + $knownIP = Sql_Fetch_Row_Query(sprintf('select * from %s where remote_ip4 = "%s" and adminid = %d ',$GLOBALS['tables']['admin_login'],$remoteAddr,$loginresult[0])); if (empty($knownIP[0])) { notifyNewIPLogin($loginresult[0]); } Sql_Query(sprintf('insert into %s (moment,adminid,remote_ip4,remote_ip6,sessionid,active) values(%d,%d,"%s","%s","%s",1)', $GLOBALS['tables']['admin_login'],time(),$loginresult[0],$remoteAddr,"",session_id())); - - - - } //If passwords are encrypted and a password recovery request was made, send mail to the admin of the given email address. } elseif (isset($_REQUEST['forgotpassword'])) { From 35427aa29e94c075e57007afe06b83523fc6c483 Mon Sep 17 00:00:00 2001 From: Michiel Dethmers Date: Sat, 9 Mar 2024 09:23:52 +0000 Subject: [PATCH 3/6] force columns to be not null --- public_html/lists/admin/structure.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/public_html/lists/admin/structure.php b/public_html/lists/admin/structure.php index a4b2bdd64..ec0fd0f0a 100644 --- a/public_html/lists/admin/structure.php +++ b/public_html/lists/admin/structure.php @@ -323,12 +323,12 @@ ), 'admin_login' => array( 'id' => array('integer not null primary key auto_increment', 'Id'), - 'moment' => array('bigint', 'epoch when it happened'), - 'adminid' => array('integer', "Admin Id"), + 'moment' => array('bigint not null', 'epoch when it happened'), + 'adminid' => array('integer not null', "Admin Id"), 'remote_ip4' => array('varchar (32) not null', 'IPv4 address'), 'remote_ip6' => array('varchar (50) not null', 'IPv6 address'), 'sessionid' => array('varchar (50) not null', 'Session ID'), - 'active' => array('tinyint', 'is this login active'), + 'active' => array('tinyint not null default 0', 'is this login active'), ), /* * obsolete tables "task" => array( From bcef815ecbbd89f177bd5229ebf41b1e42ada80f Mon Sep 17 00:00:00 2001 From: Michiel Dethmers Date: Sat, 9 Mar 2024 15:44:13 +0000 Subject: [PATCH 4/6] prevent blocking login on an non-upgraded system and send login alert just to admin, or superuser --- public_html/lists/admin/index.php | 32 +++++++++++++++++++------------ public_html/lists/admin/lib.php | 3 +-- 2 files changed, 21 insertions(+), 14 deletions(-) diff --git a/public_html/lists/admin/index.php b/public_html/lists/admin/index.php index d04ed3876..c747423e7 100644 --- a/public_html/lists/admin/index.php +++ b/public_html/lists/admin/index.php @@ -290,6 +290,7 @@ function mb_strtolower($string) } echo "$page_title"; $inRemoteCall = false; +$doLoginCheck = Sql_Table_exists($tables['admin_login']); if (!empty($GLOBALS['require_login'])) { //bth 7.1.2015 to support x-forwarded-for @@ -312,9 +313,10 @@ function mb_strtolower($string) $msg = $loginresult[1]; } else { session_regenerate_id(); - - # invalidate other active sessions - Sql_Query(sprintf('update %s set active = 0 where adminid = %d and active != 0',$GLOBALS['tables']['admin_login'],$loginresult[0])); + if ($doLoginCheck) { + # invalidate other active sessions + Sql_Query(sprintf('update %s set active = 0 where adminid = %d and active != 0',$GLOBALS['tables']['admin_login'],$loginresult[0])); + } $_SESSION['adminloggedin'] = $remoteAddr; $_SESSION['logindetails'] = array( @@ -330,14 +332,16 @@ function mb_strtolower($string) $page = preg_replace('/\W+/', '', $_POST['page']); } - # check if this is a new IP address - $knownIP = Sql_Fetch_Row_Query(sprintf('select * from %s where remote_ip4 = "%s" and adminid = %d ',$GLOBALS['tables']['admin_login'],$remoteAddr,$loginresult[0])); - if (empty($knownIP[0])) { - notifyNewIPLogin($loginresult[0]); + if ($doLoginCheck) { + # check if this is a new IP address + $knownIP = Sql_Fetch_Row_Query(sprintf('select * from %s where remote_ip4 = "%s" and adminid = %d ',$GLOBALS['tables']['admin_login'],$remoteAddr,$loginresult[0])); + if (empty($knownIP[0])) { + notifyNewIPLogin($loginresult[0]); + } + Sql_Query(sprintf('insert into %s (moment,adminid,remote_ip4,remote_ip6,sessionid,active) + values(%d,%d,"%s","%s","%s",1)', + $GLOBALS['tables']['admin_login'],time(),$loginresult[0],$remoteAddr,"",session_id())); } - Sql_Query(sprintf('insert into %s (moment,adminid,remote_ip4,remote_ip6,sessionid,active) - values(%d,%d,"%s","%s","%s",1)', - $GLOBALS['tables']['admin_login'],time(),$loginresult[0],$remoteAddr,"",session_id())); } //If passwords are encrypted and a password recovery request was made, send mail to the admin of the given email address. } elseif (isset($_REQUEST['forgotpassword'])) { @@ -386,8 +390,12 @@ function mb_strtolower($string) $_SESSION['logindetails'] = ''; $page = 'login'; } elseif ($_SESSION['adminloggedin'] && $_SESSION['logindetails']) { - $active = Sql_Fetch_Row_Query(sprintf('select active from %s where adminid = %d and (remote_ip4 = "%s" or remote_ip6 = "%s") and sessionid = "%s"', - $GLOBALS['tables']['admin_login'],$_SESSION['logindetails']['id'],$remoteAddr,"",session_id())); + if ($doLoginCheck) { + $active = Sql_Fetch_Row_Query(sprintf('select active from %s where adminid = %d and (remote_ip4 = "%s" or remote_ip6 = "%s") and sessionid = "%s"', + $GLOBALS['tables']['admin_login'],$_SESSION['logindetails']['id'],$remoteAddr,"",session_id())); + } else { + $active = array(1); ## pretend to be active + } $validate = $GLOBALS['admin_auth']->validateAccount($_SESSION['logindetails']['id']); if (empty($active[0]) || !$validate[0]) { logEvent(sprintf($GLOBALS['I18N']->get('invalidated login from %s for %s (error %s)'), $remoteAddr, diff --git a/public_html/lists/admin/lib.php b/public_html/lists/admin/lib.php index e516b5bb0..cf23cc263 100644 --- a/public_html/lists/admin/lib.php +++ b/public_html/lists/admin/lib.php @@ -2506,9 +2506,8 @@ function notifyNewIPLogin($adminId) { $GLOBALS['config']['website']); $admin_mail = $GLOBALS['admin_auth']->adminEmail($_SESSION['logindetails']['id']); - sendMail($admin_mail, $GLOBALS['installation_name'].' '.s('login from new location'), $msg, system_messageheaders($admin_mail)); + $ok = sendMail($admin_mail, $GLOBALS['installation_name'].' '.s('login from new location'), $msg, system_messageheaders($admin_mail)); - $ok = sendAdminCopy(s('login from new location'), "\n".$msg); if ($ok === 0) { $main_admin_mail = getConfig('admin_address'); logEvent(sprintf('Error sending login notification to %s', $admin_mail)); From cea08890b451905d083502d77620ad3de69ca735 Mon Sep 17 00:00:00 2001 From: Michiel Dethmers Date: Sun, 10 Mar 2024 10:10:19 +0000 Subject: [PATCH 5/6] keep newlines in translation as they are --- public_html/lists/admin/languages.php | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/public_html/lists/admin/languages.php b/public_html/lists/admin/languages.php index 183a21579..6782a5f21 100644 --- a/public_html/lists/admin/languages.php +++ b/public_html/lists/admin/languages.php @@ -377,12 +377,10 @@ public function formatText($text) if (isset($GLOBALS['developer_email'])) { if (!empty($_SESSION['show_translation_colours'])) { - return ''.str_replace("\n", '', $text).''; + return ''.$text.''; } -// return 'TE'.$text.'XT'; } -// return ''.str_replace("\n","",$text).''; - return str_replace("\n", '', $text); + return $text; } /** From 7c0e6c53b82c72fcda554d291018980227391787 Mon Sep 17 00:00:00 2001 From: Michiel Dethmers Date: Sun, 10 Mar 2024 10:12:31 +0000 Subject: [PATCH 6/6] make shorter lines, so it renders a bit better --- public_html/lists/admin/lib.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/public_html/lists/admin/lib.php b/public_html/lists/admin/lib.php index cf23cc263..2ddcf5b29 100644 --- a/public_html/lists/admin/lib.php +++ b/public_html/lists/admin/lib.php @@ -2499,8 +2499,9 @@ function notifyNewIPLogin($adminId) { -------------------------------------------------------------------------------- We noticed a login to your phpList installation at https://%s -from a new location. If this was you, you can delete this message. If you do not recognise -this, please login to your phpList installation and change your password. +from a new location. If this was you, you can delete this message. +If you do not recognise this, please login to your phpList installation +and change your password. -------------------------------------------------------------------------------- ', $GLOBALS['config']['website']);