From f853f6523fbf5896816843c5a519bd033e54a257 Mon Sep 17 00:00:00 2001 From: Marc Alexander Date: Sun, 28 Apr 2024 13:46:25 +0200 Subject: [PATCH] [ticket/security/276] Prevent sending activation emails multiple times per day SECURITY-276 --- phpBB/includes/acp/acp_inactive.php | 5 ++-- phpBB/includes/acp/acp_users.php | 18 ++++++++----- phpBB/includes/functions_user.php | 23 ++++++++-------- phpBB/includes/ucp/ucp_profile.php | 7 ++--- phpBB/includes/ucp/ucp_register.php | 25 ++++++++--------- phpBB/includes/ucp/ucp_resend.php | 27 +++++++++++++++++++ phpBB/install/schemas/schema_data.sql | 4 +-- phpBB/phpbb/console/command/user/add.php | 34 +++++++++++++++++++++--- 8 files changed, 103 insertions(+), 40 deletions(-) diff --git a/phpBB/includes/acp/acp_inactive.php b/phpBB/includes/acp/acp_inactive.php index 7b4536f755..b85ed86ee5 100644 --- a/phpBB/includes/acp/acp_inactive.php +++ b/phpBB/includes/acp/acp_inactive.php @@ -238,10 +238,11 @@ class acp_inactive $messenger->save_queue(); - // Add the remind state to the database + // Add the remind state to the database and increase activation expiration by one day $sql = 'UPDATE ' . USERS_TABLE . ' SET user_reminded = user_reminded + 1, - user_reminded_time = ' . time() . ' + user_reminded_time = ' . time() . ', + user_actkey_expiration = ' . (int) strtotime('+1 day') . ' WHERE ' . $db->sql_in_set('user_id', $user_ids); $db->sql_query($sql); diff --git a/phpBB/includes/acp/acp_users.php b/phpBB/includes/acp/acp_users.php index 7c3468f90d..f54ff08db4 100644 --- a/phpBB/includes/acp/acp_users.php +++ b/phpBB/includes/acp/acp_users.php @@ -385,14 +385,18 @@ class acp_users $user_actkey = empty($user_activation_key) ? $user_actkey : $user_activation_key; } - if ($user_row['user_type'] == USER_NORMAL || empty($user_activation_key)) - { - $sql = 'UPDATE ' . USERS_TABLE . " - SET user_actkey = '" . $db->sql_escape($user_actkey) . "' - WHERE user_id = $user_id"; - $db->sql_query($sql); - } + // Always update actkey even if same and also update actkey expiration to 24 hours from now + $sql_ary = [ + 'user_actkey' => $user_actkey, + 'user_actkey_expiration' => strtotime('+1 day'), + ]; + $sql = 'UPDATE ' . USERS_TABLE . ' + SET ' . $db->sql_build_array('UPDATE', $sql_ary) . ' + WHERE user_id = ' . $user_id; + $db->sql_query($sql); + + // Start sending email $messenger = new messenger(false); $messenger->template($email_template, $user_row['user_lang']); diff --git a/phpBB/includes/functions_user.php b/phpBB/includes/functions_user.php index 25adf4215a..c33caba8e8 100644 --- a/phpBB/includes/functions_user.php +++ b/phpBB/includes/functions_user.php @@ -210,18 +210,19 @@ function user_add($user_row, $cp_data = false, $notifications_data = null) // These are the additional vars able to be specified $additional_vars = array( - 'user_permissions' => '', - 'user_timezone' => $config['board_timezone'], - 'user_dateformat' => $config['default_dateformat'], - 'user_lang' => $config['default_lang'], - 'user_style' => (int) $config['default_style'], - 'user_actkey' => '', - 'user_ip' => '', - 'user_regdate' => time(), - 'user_passchg' => time(), - 'user_options' => 230271, + 'user_permissions' => '', + 'user_timezone' => $config['board_timezone'], + 'user_dateformat' => $config['default_dateformat'], + 'user_lang' => $config['default_lang'], + 'user_style' => (int) $config['default_style'], + 'user_actkey' => '', + 'user_actkey_expiration' => 0, + 'user_ip' => '', + 'user_regdate' => time(), + 'user_passchg' => time(), + 'user_options' => 230271, // We do not set the new flag here - registration scripts need to specify it - 'user_new' => 0, + 'user_new' => 0, 'user_inactive_reason' => 0, 'user_inactive_time' => 0, diff --git a/phpBB/includes/ucp/ucp_profile.php b/phpBB/includes/ucp/ucp_profile.php index 4a8abcd46a..616792d9ac 100644 --- a/phpBB/includes/ucp/ucp_profile.php +++ b/phpBB/includes/ucp/ucp_profile.php @@ -196,9 +196,10 @@ class ucp_profile { $notifications_manager = $phpbb_container->get('notification_manager'); $notifications_manager->add_notifications('notification.type.admin_activate_user', array( - 'user_id' => $user->data['user_id'], - 'user_actkey' => $user_actkey, - 'user_regdate' => time(), // Notification time + 'user_id' => $user->data['user_id'], + 'user_actkey' => $user_actkey, + 'user_actkey_expiration' => strtotime('+1 day'), // 24 hours until activation can be resent + 'user_regdate' => time(), // Notification time )); } diff --git a/phpBB/includes/ucp/ucp_register.php b/phpBB/includes/ucp/ucp_register.php index 5f7a87f74b..4d6677567a 100644 --- a/phpBB/includes/ucp/ucp_register.php +++ b/phpBB/includes/ucp/ucp_register.php @@ -381,18 +381,19 @@ class ucp_register $passwords_manager = $phpbb_container->get('passwords.manager'); $user_row = array( - 'username' => $data['username'], - 'user_password' => $passwords_manager->hash($data['new_password']), - 'user_email' => $data['email'], - 'group_id' => (int) $group_id, - 'user_timezone' => $data['tz'], - 'user_lang' => $data['lang'], - 'user_type' => $user_type, - 'user_actkey' => $user_actkey, - 'user_ip' => $user->ip, - 'user_regdate' => time(), - 'user_inactive_reason' => $user_inactive_reason, - 'user_inactive_time' => $user_inactive_time, + 'username' => $data['username'], + 'user_password' => $passwords_manager->hash($data['new_password']), + 'user_email' => $data['email'], + 'group_id' => (int) $group_id, + 'user_timezone' => $data['tz'], + 'user_lang' => $data['lang'], + 'user_type' => $user_type, + 'user_actkey' => $user_actkey, + 'user_actkey_expiration' => strtotime('+1 day'), // 24 hours until activation can be resent + 'user_ip' => $user->ip, + 'user_regdate' => time(), + 'user_inactive_reason' => $user_inactive_reason, + 'user_inactive_time' => $user_inactive_time, ); if ($config['new_member_post_limit']) diff --git a/phpBB/includes/ucp/ucp_resend.php b/phpBB/includes/ucp/ucp_resend.php index 7f952af6c3..6ff1e53027 100644 --- a/phpBB/includes/ucp/ucp_resend.php +++ b/phpBB/includes/ucp/ucp_resend.php @@ -73,6 +73,12 @@ class ucp_resend trigger_error('ACCOUNT_DEACTIVATED'); } + // Do not resend activation email if valid one still exists + if (!empty($user_row['user_actkey']) && (int) $user_row['user_actkey_expiration'] >= time()) + { + trigger_error('ACTIVATION_EMAIL_ALREADY_SENT'); + } + // Determine coppa status on group (REGISTERED(_COPPA)) $sql = 'SELECT group_name, group_type FROM ' . GROUPS_TABLE . ' @@ -144,6 +150,8 @@ class ucp_resend $db->sql_freeresult($result); } + $this->update_activation_expiration(); + meta_refresh(3, append_sid("{$phpbb_root_path}index.$phpEx")); $message = ($config['require_activation'] == USER_ACTIVATION_ADMIN) ? $user->lang['ACTIVATION_EMAIL_SENT_ADMIN'] : $user->lang['ACTIVATION_EMAIL_SENT']; @@ -160,4 +168,23 @@ class ucp_resend $this->tpl_name = 'ucp_resend'; $this->page_title = 'UCP_RESEND'; } + + /** + * Update activation expiration to 1 day from now + * + * @return void + */ + protected function update_activation_expiration(): void + { + global $db, $user; + + $sql_ary = [ + 'user_actkey_expiration' => strtotime('+1 day'), + ]; + + $sql = 'UPDATE ' . USERS_TABLE . ' + SET ' . $db->sql_build_array('UPDATE', $sql_ary) . ' + WHERE user_id = ' . (int) $user->id(); + $db->sql_query($sql); + } } diff --git a/phpBB/install/schemas/schema_data.sql b/phpBB/install/schemas/schema_data.sql index 7ec43e3b54..a9c8d4a049 100644 --- a/phpBB/install/schemas/schema_data.sql +++ b/phpBB/install/schemas/schema_data.sql @@ -527,10 +527,10 @@ INSERT INTO phpbb_forums (forum_name, forum_desc, left_id, right_id, parent_id, INSERT INTO phpbb_forums (forum_name, forum_desc, left_id, right_id, parent_id, forum_type, forum_posts_approved, forum_posts_unapproved, forum_posts_softdeleted, forum_topics_approved, forum_topics_unapproved, forum_topics_softdeleted, forum_last_post_id, forum_last_poster_id, forum_last_poster_name, forum_last_poster_colour, forum_last_post_subject, forum_last_post_time, forum_link, forum_password, forum_image, forum_rules, forum_rules_link, forum_rules_uid, forum_desc_uid, prune_freq, prune_days, prune_viewed, forum_parents, forum_flags) VALUES ('{L_FORUMS_TEST_FORUM_TITLE}', '{L_FORUMS_TEST_FORUM_DESC}', 2, 3, 1, 1, 1, 0, 0, 1, 0, 0, 1, 2, 'Admin', 'AA0000', '{L_TOPICS_TOPIC_TITLE}', 972086460, '', '', '', '', '', '', '', 1, 7, 7, '', 48); # -- Users / Anonymous user -INSERT INTO phpbb_users (user_type, group_id, username, username_clean, user_regdate, user_password, user_email, user_lang, user_style, user_rank, user_colour, user_posts, user_permissions, user_ip, user_birthday, user_lastpage, user_last_confirm_key, user_post_sortby_type, user_post_sortby_dir, user_topic_sortby_type, user_topic_sortby_dir, user_avatar, user_sig, user_sig_bbcode_uid, user_jabber, user_actkey, user_newpasswd, user_allow_massemail) VALUES (2, 1, 'Anonymous', 'anonymous', 0, '', '', 'en', 1, 0, '', 0, '', '', '', '', '', 't', 'a', 't', 'd', '', '', '', '', '', '', 0); +INSERT INTO phpbb_users (user_type, group_id, username, username_clean, user_regdate, user_password, user_email, user_lang, user_style, user_rank, user_colour, user_posts, user_permissions, user_ip, user_birthday, user_lastpage, user_last_confirm_key, user_post_sortby_type, user_post_sortby_dir, user_topic_sortby_type, user_topic_sortby_dir, user_avatar, user_sig, user_sig_bbcode_uid, user_jabber, user_actkey, user_actkey_expiration, user_newpasswd, user_allow_massemail) VALUES (2, 1, 'Anonymous', 'anonymous', 0, '', '', 'en', 1, 0, '', 0, '', '', '', '', '', 't', 'a', 't', 'd', '', '', '', '', '', 0, '', 0); # -- username: Admin password: admin (change this or remove it once everything is working!) -INSERT INTO phpbb_users (user_type, group_id, username, username_clean, user_regdate, user_password, user_email, user_lang, user_style, user_rank, user_colour, user_posts, user_permissions, user_ip, user_birthday, user_lastpage, user_last_confirm_key, user_post_sortby_type, user_post_sortby_dir, user_topic_sortby_type, user_topic_sortby_dir, user_avatar, user_sig, user_sig_bbcode_uid, user_jabber, user_actkey, user_newpasswd) VALUES (3, 5, 'Admin', 'admin', 0, '21232f297a57a5a743894a0e4a801fc3', 'admin@yourdomain.com', 'en', 1, 1, 'AA0000', 1, '', '', '', '', '', 't', 'a', 't', 'd', '', '', '', '', '', ''); +INSERT INTO phpbb_users (user_type, group_id, username, username_clean, user_regdate, user_password, user_email, user_lang, user_style, user_rank, user_colour, user_posts, user_permissions, user_ip, user_birthday, user_lastpage, user_last_confirm_key, user_post_sortby_type, user_post_sortby_dir, user_topic_sortby_type, user_topic_sortby_dir, user_avatar, user_sig, user_sig_bbcode_uid, user_jabber, user_actkey, user_actkey_expiration, user_newpasswd) VALUES (3, 5, 'Admin', 'admin', 0, '21232f297a57a5a743894a0e4a801fc3', 'admin@yourdomain.com', 'en', 1, 1, 'AA0000', 1, '', '', '', '', '', 't', 'a', 't', 'd', '', '', '', '', '', 0, ''); # -- Groups INSERT INTO phpbb_groups (group_name, group_type, group_founder_manage, group_colour, group_legend, group_avatar, group_desc, group_desc_uid, group_max_recipients) VALUES ('GUESTS', 3, 0, '', 0, '', '', '', 5); diff --git a/phpBB/phpbb/console/command/user/add.php b/phpBB/phpbb/console/command/user/add.php index 40f866c176..334ff71415 100644 --- a/phpBB/phpbb/console/command/user/add.php +++ b/phpBB/phpbb/console/command/user/add.php @@ -290,18 +290,17 @@ class add extends command { case USER_ACTIVATION_SELF: $email_template = 'user_welcome_inactive'; - $user_actkey = gen_rand_string(mt_rand(6, 10)); break; case USER_ACTIVATION_ADMIN: $email_template = 'admin_welcome_inactive'; - $user_actkey = gen_rand_string(mt_rand(6, 10)); break; default: $email_template = 'user_welcome'; - $user_actkey = ''; break; } + $user_actkey = $this->get_activation_key($user_id); + if (!class_exists('messenger')) { require($this->phpbb_root_path . 'includes/functions_messenger.' . $this->php_ext); @@ -321,6 +320,35 @@ class add extends command $messenger->send(NOTIFY_EMAIL); } + /** + * Get user activation key + * + * @param int $user_id User ID + * + * @return string User activation key for user + */ + protected function get_activation_key(int $user_id): string + { + $user_actkey = ''; + + if ($this->config['require_activation'] == USER_ACTIVATION_SELF || $this->config['require_activation'] == USER_ACTIVATION_ADMIN) + { + $user_actkey = gen_rand_string(mt_rand(6, 10)); + + $sql_ary = [ + 'user_actkey' => $user_actkey, + 'user_actkey_expiration' => strtotime('+1 day'), + ]; + + $sql = 'UPDATE ' . USERS_TABLE . ' + SET ' . $this->db->sql_build_array('UPDATE', $sql_ary) . ' + WHERE user_id = ' . $user_id; + $this->db->sql_query($sql); + } + + return $user_actkey; + } + /** * Helper to translate questions to the user *