2
\$\begingroup\$

This is a function that checks that when users register into the site, they only create one single account per email. The little twist is that all emails in the DB are hashed with unique salts (=Input2). Because hashing the new mail input with all unique salts (=STEP 2) will be horribly slow, I keep a "diggest" version of emails (=Input3) in my DB. The diggest emails are a shorter and useless version of the emails. For example, [email protected] becomes [email protected]. The goal of that is diggest version is to reduce the number of emails to check in the DB (=STEP 1).

This function works pretty well but I'm afraid it will became very slow when DB becomes bigger.

Here is the code of the portion relevant to the problem:

$Requete1 = mysql_query("SELECT Input2 as email_test, Salt as salt FROM utilisateurs WHERE Input3='$email_diggest'");

    //Step 1
    if(mysql_num_rows($Requete1) == 0)
    {
        echo "No problem baby";
    }

    //Step 2
    while($row = mysql_fetch_assoc($Requete1))
    {
        $email_test = $row['email_test'];
        $salt = $row['salt'];

        $hash_email = base64_encode(pbkdf2($Email, $salt, 100000, 32));

        if($email_test == $hash_email)
        {
            echo "An account already exist with that email";
        }else
        {
            echo "No problem baby";
        }
    }
\$\endgroup\$
3
  • 1
    \$\begingroup\$ Why would you hash the email? If you need the email in plain text, just keep the email in plain text. Emails aren't particularly sensitive information. \$\endgroup\$
    – Corbin
    Commented Aug 18, 2012 at 9:11
  • \$\begingroup\$ You can see why here: programmers.stackexchange.com/questions/161301/… \$\endgroup\$ Commented Aug 18, 2012 at 9:13
  • \$\begingroup\$ I have to agree with the side that was responding that you've gone a step too far. A compromised email address (not email address and password) is by no means the end of the world. Nevertheless, later I'll write a full response if no one beats me to it. \$\endgroup\$
    – Corbin
    Commented Aug 18, 2012 at 9:26

1 Answer 1

1
\$\begingroup\$

I have to agree with Corbin, while I would be annoyed if my email address got leaked, I would not be overly concerned. Spam filters are pretty decent now, and without a password all they can do is spam me. Or, if I have a gmail account, WoW hackers can find more loopholes that allows them to spam FROM my address. I don't even play WoW anymore, I wish they'd just leave me alone -.- Anyways...

This looks like sample code, so there's not much I can help with. I would suggest that you be sure to escape early whenever possible. For instance, in the first if statement. This ensures there is no wasted processes parsing code that ins't necessary. Also, not sure how much of a difference this would make, but I believe your email comparison should be absolute, not loose.

if( $email_test === $hash_email ) {

Only other thing I can think of is using extract() to get those array indices. But there's a couple of problems with that. First, its not always well liked by the community. Second, it dumps the entire array index list into the function's local variable scope, which is sometimes unnecessary and may lead to security issues. I only bring this up because if these are the only two fields in this table then there shouldn't be any issues here. As for speed, I'm not sure, you'd have to profile it.

The biggest thing here is premature optimization. Yes, its nice to plan ahead, but ask yourself this: How likely is it that your database will ever reach a size where this will become an issue? Every developer dreams that their creation will be the next biggest thing and will have millions of viewers, but the likelihood is slim. Plus, with regular maintenance and periodic deletion of inactive accounts will help ensure that the size, even in these extreme cases, is less likely to become a factor.

\$\endgroup\$
1
  • \$\begingroup\$ You sir, have just made a very good point. \$\endgroup\$ Commented Aug 21, 2012 at 9:26

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.