WeBid Bug Tracking

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0000275WeBidRegistrationpublic2011-05-16 06:012011-06-12 19:03
ReporterLunkwill 
Assigned To 
PrioritynormalSeverityminorReproducibilityhave not tried
StatusresolvedResolutionfixed 
PlatformOSOS Version
Product Version1.0.2 
Target VersionFixed in Version1.0.3 
Summary0000275: Registration confirm link is guessable
DescriptionConfirmation emails are supposed to verify that the new user at least entered a correct email address they can read. That only works if they get sent a confirmation link they can't trivially guess. Knowing the hash value in the link is md5($nick), I can create a bunch of fake users and "confirm" their addresses by just calculating the MD5 myself.

As we already have an $MD5_PREFIX variable that's supposed to be random for each site, the fix is easy: just concatenate it with the nick in includes/user_confirmation_needapproval.inc.php, includes/user_confirmation.inc.php, confirm.php and admin/listusers.php



Here's a diff against r293 from SVN - untested as I've made too many modifications in the version I actually run, but unless I overlooked some place where the concatenation is needed it's trivial.



Index: confirm.php

===================================================================

--- confirm.php (revision 293)

+++ confirm.php (working copy)

@@ -23,7 +23,7 @@

        {

                $errmsg = $ERR_025;

        }

- elseif (!isset($_GET['hash']) || md5($system->uncleanvars(mysql_result($result, 0, 'nick'))) != $_GET['hash'])

+ elseif (!isset($_GET['hash']) || md5($MD5_PREFIX . $system->uncleanvars(mysql_result($result, 0, 'nick'))) != $_GET['hash'])

        {

                $errmsg = $ERR_033;

        }

@@ -57,7 +57,7 @@

        $query = "SELECT nick FROM " . $DBPrefix . "users WHERE id = " . intval($_POST['id']);

        $res = mysql_query($query);

        $system->check_mysql($res, $query, __LINE__, __FILE__);

- if (md5(mysql_result($res, 0, 'nick')) == $_POST['hash'])

+ if (md5($MD5_PREFIX . mysql_result($res, 0, 'nick')) == $_POST['hash'])

        {

                // User wants to confirm his/her registration

                $query = "UPDATE " . $DBPrefix . "users SET suspended = 0 WHERE id = " . intval($_POST['id']) . " AND suspended = 8";

@@ -106,7 +106,7 @@

        $query = "SELECT nick FROM " . $DBPrefix . "users WHERE id = " . intval($_POST['id']);

        $res = mysql_query($query);

        $system->check_mysql($res, $query, __LINE__, __FILE__);

- if (md5(mysql_result($res, 0, 'nick')) == $_POST['hash'])

+ if (md5($MD5_PREFIX . mysql_result($res, 0, 'nick')) == $_POST['hash'])

        {

                // User doesn't want to confirm hid/her registration

                $query = "DELETE FROM " . $DBPrefix . "users WHERE id = " . intval($_POST['id']) . " AND suspended = 8";

Index: includes/user_confirmation_needapproval.inc.php

===================================================================

--- includes/user_confirmation_needapproval.inc.php (revision 293)

+++ includes/user_confirmation_needapproval.inc.php (working copy)

@@ -30,10 +30,10 @@

                'SITENAME' => $system->SETTINGS['sitename'],

                'SITEURL' => $system->SETTINGS['siteurl'],

                'ADMINEMAIL' => $system->SETTINGS['adminmail'],

- 'CONFIRMATION_PAGE' => $system->SETTINGS['siteurl'] . 'confirm.php?id=' . $TPL_id_hidden . '&hash=' . md5($TPL_nick_hidden),

+ 'CONFIRMATION_PAGE' => $system->SETTINGS['siteurl'] . 'confirm.php?id=' . $TPL_id_hidden . '&hash=' . md5($MD5_PREFIX . $TPL_nick_hidden),

                'LOGO' => $system->SETTINGS['siteurl'] . 'themes/' . $system->SETTINGS['theme'] . '/' . $system->SETTINGS['logo']

                ));

 $emailer->email_uid = $TPL_id_hidden;

 $emailer->email_sender(array($TPL_email_hidden, $system->SETTINGS['adminmail']), 'user_needapproval.inc.php', $system->SETTINGS['sitename']. ' '.$MSG['098']);

 

-?>

\ No newline at end of file

+?>

Index: includes/user_confirmation.inc.php

===================================================================

--- includes/user_confirmation.inc.php (revision 293)

+++ includes/user_confirmation.inc.php (working copy)

@@ -19,7 +19,7 @@

                'SITENAME' => $system->SETTINGS['sitename'],

                'SITEURL' => $system->SETTINGS['siteurl'],

                'ADMINMAIL' => $system->SETTINGS['adminmail'],

- 'CONFIRMURL' => $system->SETTINGS['siteurl'] . 'confirm.php?id=' . $TPL_id_hidden . '&hash=' . md5($TPL_nick_hidden),

+ 'CONFIRMURL' => $system->SETTINGS['siteurl'] . 'confirm.php?id=' . $TPL_id_hidden . '&hash=' . md5($MD5_PREFIX . $TPL_nick_hidden),

                'C_NAME' => $TPL_name_hidden

                ));

 $emailer->email_uid = $TPL_id_hidden;

Index: admin/listusers.php

===================================================================

--- admin/listusers.php (revision 293)

+++ admin/listusers.php (working copy)

@@ -34,7 +34,7 @@

                                'SITENAME' => $system->SETTINGS['sitename'],

                                'SITEURL' => $system->SETTINGS['siteurl'],

                                'ADMINMAIL' => $system->SETTINGS['adminmail'],

- 'CONFIRMURL' => $system->SETTINGS['siteurl'] . 'confirm.php?id=' . $USER['id'] . '&hash=' . md5($USER['nick']),

+ 'CONFIRMURL' => $system->SETTINGS['siteurl'] . 'confirm.php?id=' . $USER['id'] . '&hash=' . md5($MD5_PREFIX . $USER['nick']),

                                'C_NAME' => $USER['name']

                                ));

                $emailer->email_uid = $USER['id'];
TagsNo tags attached.
import_id277
Thread
Attached Files

- Relationships

-  Notes
(0000719)
renlok (administrator)
2011-06-03 09:18
edited on: 1970-01-01 00:00

Cheers for this I've added it for next release, Im also going to add a feature so it generates a new code at install or something like that
(0000721)
Acden (viewer)
2011-06-04 19:50
edited on: 1970-01-01 00:00

Really required feature because it is connected with security.
(0000740)
Lunkwill (reporter)
2011-06-12 19:03
edited on: 1970-01-01 00:00

Well, it's just a minor security issue as it only saves a fraudster the hassle of setting up a bunch of fake freemail accounts using TOR or something similar. But nice that Renlok has integrated it already :)

- Issue History
Date Modified Username Field Change
2015-04-01 13:17 renlok New Issue
2015-04-01 13:17 renlok import_id => 277
2015-04-01 13:17 renlok Date Submitted 2015-04-01 13:17 => 2011-05-16 06:01
2015-04-01 13:17 renlok Last Update 2015-04-01 13:17 => 2011-06-12 19:03


Copyright © 2000 - 2018 MantisBT Team
Powered by Mantis Bugtracker