Skip to content

Commit

Permalink
bug 3050: increase security on reset password algorithm.
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@plegall
plegall committed Jul 28, 2014
1 parent dc69d64 commit 87a30ff
Show file tree
Hide file tree
Showing 3 changed files with 126 additions and 77 deletions.
24 changes: 0 additions & 24 deletions include/functions_user.inc.php
View file
Expand Up @@ -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;
}
}
}

?>
43 changes: 43 additions & 0 deletions install/db/144-database.php
View file
@@ -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";

?>
136 changes: 83 additions & 53 deletions password.php
View file
Expand Up @@ -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();

Expand All @@ -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();
Expand Down Expand Up @@ -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;
}

/**
Expand All @@ -187,33 +225,24 @@ function check_password_reset_key($key)
*/
function reset_password()
{
global $page, $user, $conf;
global $page, $conf;

if ($_POST['use_new_pwd'] != $_POST['passwordConf'])
{
$page['errors'][] = l10n('The passwords do not match');
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(
Expand All @@ -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;
}
Expand Down Expand Up @@ -270,7 +300,7 @@ function reset_password()
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))
Expand Down

0 comments on commit 87a30ff

Please sign in to comment.