aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorplegall <plg@piwigo.org>2014-07-28 19:27:50 +0000
committerplegall <plg@piwigo.org>2014-07-28 19:27:50 +0000
commit87a30ff064c14ecdac1dd7a67c237a7299312bd5 (patch)
tree60422017450a23f8c10d9a77977be7da5ce2205f
parentdc69d64bb0dbe44715f8d39202419354b251e2a9 (diff)
bug 3050: increase security on reset password algorithm.
* reset key has a 1-hour life * reset key is automatically deleted once used * reset key is stored as a hash Thank you effigies for code suggestions git-svn-id: http://piwigo.org/svn/trunk@29111 68402e56-0260-453c-a942-63ccdbb3a9ee
Diffstat (limited to '')
-rw-r--r--include/functions_user.inc.php24
-rw-r--r--install/db/144-database.php43
-rw-r--r--password.php136
3 files changed, 126 insertions, 77 deletions
diff --git a/include/functions_user.inc.php b/include/functions_user.inc.php
index f91f3f8fe..126a22838 100644
--- a/include/functions_user.inc.php
+++ b/include/functions_user.inc.php
@@ -1466,28 +1466,4 @@ function get_recent_photos_sql($db_field)
.pwg_db_get_recent_period_expression($user['recent_period'])
.','.pwg_db_get_recent_period_expression(1,$user['last_photo_date']).')';
}
-
-/**
- * Returns a unique activation key.
- *
- * @return string
- */
-function get_user_activation_key()
-{
- while (true)
- {
- $key = generate_key(20);
- $query = '
-SELECT COUNT(*)
- FROM '.USER_INFOS_TABLE.'
- WHERE activation_key = \''.$key.'\'
-;';
- list($count) = pwg_db_fetch_row(pwg_query($query));
- if (0 == $count)
- {
- return $key;
- }
- }
-}
-
?> \ No newline at end of file
diff --git a/install/db/144-database.php b/install/db/144-database.php
new file mode 100644
index 000000000..6f1e7b936
--- /dev/null
+++ b/install/db/144-database.php
@@ -0,0 +1,43 @@
+<?php
+// +-----------------------------------------------------------------------+
+// | Piwigo - a PHP based photo gallery |
+// +-----------------------------------------------------------------------+
+// | Copyright(C) 2008-2014 Piwigo Team http://piwigo.org |
+// | Copyright(C) 2003-2008 PhpWebGallery Team http://phpwebgallery.net |
+// | Copyright(C) 2002-2003 Pierrick LE GALL http://le-gall.net/pierrick |
+// +-----------------------------------------------------------------------+
+// | This program is free software; you can redistribute it and/or modify |
+// | it under the terms of the GNU General Public License as published by |
+// | the Free Software Foundation |
+// | |
+// | This program is distributed in the hope that it will be useful, but |
+// | WITHOUT ANY WARRANTY; without even the implied warranty of |
+// | MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU |
+// | General Public License for more details. |
+// | |
+// | You should have received a copy of the GNU General Public License |
+// | along with this program; if not, write to the Free Software |
+// | Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, |
+// | USA. |
+// +-----------------------------------------------------------------------+
+
+if (!defined('PHPWG_ROOT_PATH'))
+{
+ die('Hacking attempt!');
+}
+
+$upgrade_description = 'add activation_key_expire';
+
+// we use PREFIX_TABLE, in case Piwigo uses an external user table
+pwg_query('
+ALTER TABLE '.USER_INFOS_TABLE.'
+ CHANGE activation_key activation_key VARCHAR(255) DEFAULT NULL,
+ ADD COLUMN activation_key_expire DATETIME DEFAULT NULL AFTER activation_key
+;');
+
+// purge current expiration keys
+pwg_query('UPDATE '.USER_INFOS_TABLE.' SET activation_key = NULL;');
+
+echo "\n".$upgrade_description."\n";
+
+?>
diff --git a/password.php b/password.php
index 90638e317..5bc94c0cb 100644
--- a/password.php
+++ b/password.php
@@ -89,18 +89,20 @@ function process_password_request()
return false;
}
- if (empty($userdata['activation_key']))
- {
- $activation_key = get_user_activation_key();
+ $activation_key = generate_key(20);
- single_update(
- USER_INFOS_TABLE,
- array('activation_key' => $activation_key),
- array('user_id' => $user_id)
- );
+ list($expire) = pwg_db_fetch_row(pwg_query('SELECT ADDDATE(NOW(), INTERVAL 1 HOUR)'));
- $userdata['activation_key'] = $activation_key;
- }
+ single_update(
+ USER_INFOS_TABLE,
+ array(
+ 'activation_key' => pwg_password_hash($activation_key),
+ 'activation_key_expire' => $expire,
+ ),
+ array('user_id' => $user_id)
+ );
+
+ $userdata['activation_key'] = $activation_key;
set_make_full_url();
@@ -112,7 +114,8 @@ function process_password_request()
);
$message.= "\r\n\r\n";
$message.= l10n('To reset your password, visit the following address:') . "\r\n";
- $message.= get_gallery_home_url().'/password.php?key='.$userdata['activation_key']."\r\n\r\n";
+ $message.= get_gallery_home_url().'/password.php?key='.$activation_key.'-'.urlencode($userdata['email']);
+ $message.= "\r\n\r\n";
$message.= l10n('If this was a mistake, just ignore this email and nothing will happen.')."\r\n";
unset_make_full_url();
@@ -143,40 +146,75 @@ function process_password_request()
*
* @return mixed (user_id if OK, false otherwise)
*/
-function check_password_reset_key($key)
+function check_password_reset_key($reset_key)
{
- global $page;
-
+ global $page, $conf;
+
+ list($key, $email) = explode('-', $reset_key, 2);
+
if (!preg_match('/^[a-z0-9]{20}$/i', $key))
{
$page['errors'][] = l10n('Invalid key');
return false;
}
+ $user_ids = array();
+
$query = '
SELECT
- user_id,
- status
- FROM '.USER_INFOS_TABLE.'
- WHERE activation_key = \''.$key.'\'
+ '.$conf['user_fields']['id'].' AS id
+ FROM '.USERS_TABLE.'
+ WHERE '.$conf['user_fields']['email'].' = \''.pwg_db_real_escape_string($email).'\'
;';
- $result = pwg_query($query);
+ $user_ids = query2array($query, null, 'id');
- if (pwg_db_num_rows($result) == 0)
+ if (count($user_ids) == 0)
{
- $page['errors'][] = l10n('Invalid key');
+ $page['errors'][] = l10n('Invalid username or email');
return false;
}
+
+ $user_id = null;
- $userdata = pwg_db_fetch_assoc($result);
+ $query = '
+SELECT
+ user_id,
+ status,
+ activation_key,
+ activation_key_expire,
+ NOW() AS dbnow
+ FROM '.USER_INFOS_TABLE.'
+ WHERE user_id IN ('.implode(',', $user_ids).')
+;';
+ $result = pwg_query($query);
+ while ($row = pwg_db_fetch_assoc($result))
+ {
+ if (pwg_password_verify($key, $row['activation_key']))
+ {
+ if (strtotime($row['dbnow']) > strtotime($row['activation_key_expire']))
+ {
+ // key has expired
+ $page['errors'][] = l10n('Invalid key');
+ return false;
+ }
+
+ if (is_a_guest($row['status']) or is_generic($row['status']))
+ {
+ $page['errors'][] = l10n('Password reset is not allowed for this user');
+ return false;
+ }
+
+ $user_id = $row['user_id'];
+ }
+ }
- if (is_a_guest($userdata['status']) or is_generic($userdata['status']))
+ if (empty($user_id))
{
- $page['errors'][] = l10n('Password reset is not allowed for this user');
+ $page['errors'][] = l10n('Invalid key');
return false;
}
-
- return $userdata['user_id'];
+
+ return $user_id;
}
/**
@@ -187,7 +225,7 @@ SELECT
*/
function reset_password()
{
- global $page, $user, $conf;
+ global $page, $conf;
if ($_POST['use_new_pwd'] != $_POST['passwordConf'])
{
@@ -195,25 +233,16 @@ function reset_password()
return false;
}
- if (isset($_GET['key']))
+ if (!isset($_GET['key']))
{
- $user_id = check_password_reset_key($_GET['key']);
- if (!is_numeric($user_id))
- {
- $page['errors'][] = l10n('Invalid key');
- return false;
- }
+ $page['errors'][] = l10n('Invalid key');
}
- else
+
+ $user_id = check_password_reset_key($_GET['key']);
+
+ if (!is_numeric($user_id))
{
- // we check the currently logged in user
- if (is_a_guest() or is_generic())
- {
- $page['errors'][] = l10n('Password reset is not allowed for this user');
- return false;
- }
-
- $user_id = $user['id'];
+ return false;
}
single_update(
@@ -222,16 +251,17 @@ function reset_password()
array($conf['user_fields']['id'] => $user_id)
);
- $page['infos'][] = l10n('Your password has been reset');
+ single_update(
+ USER_INFOS_TABLE,
+ array(
+ 'activation_key' => null,
+ 'activation_key_expire' => null,
+ ),
+ array('user_id' => $user_id)
+ );
- if (isset($_GET['key']))
- {
- $page['infos'][] = '<a href="'.get_root_url().'identification.php">'.l10n('Login').'</a>';
- }
- else
- {
- $page['infos'][] = '<a href="'.get_gallery_home_url().'">'.l10n('Return to home page').'</a>';
- }
+ $page['infos'][] = l10n('Your password has been reset');
+ $page['infos'][] = '<a href="'.get_root_url().'identification.php">'.l10n('Login').'</a>';
return true;
}
@@ -270,7 +300,7 @@ if (isset($_GET['key']) and !is_a_guest())
unset($_GET['key']);
}
-if (isset($_GET['key']))
+if (isset($_GET['key']) and !isset($_POST['submit']))
{
$user_id = check_password_reset_key($_GET['key']);
if (is_numeric($user_id))