1. Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Feb 2013
    Posts
    392
    Rep Power
    8
    ok, basically it works, now I'll try to make it request 1 time in 30 minutes (this way I think there is enough room for error for the user). I will post them when done
  2. --
    Devshed Expert (3500 - 3999 posts)

    Join Date
    Jul 2012
    Posts
    3,957
    Rep Power
    1046
    You mean limit the number of emails to 1 in 30 minutes? That makes no sense to me.

    You don't forget your password several times a day. You forget it once, and then you want to recover your account as soons as possible. If the first email somehow didn't get through, you wanna try again immediately without having to wait 30 minutes or something.

    So rather than putting a delay between each request, you should limit the number of emails to, say, 10 in 24 hours. This is more user-friendly and still provides protection against email flooding. You might wanna put a small delay of, say, 1 minute between each request to prevent people from clicking on the button 10 times.
    The 6 worst sins of security ē How to (properly) access a MySQL database with PHP

    Why canít I use certain words like "drop" as part of my Security Question answers?
    There are certain words used by hackers to try to gain access to systems and manipulate data; therefore, the following words are restricted: "select," "delete," "update," "insert," "drop" and "null".
  3. Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Feb 2013
    Posts
    392
    Rep Power
    8
    Jacques, do you know how I can check if a datetime already occures in a table, something like:

    PHP Code:
    $time = new DateTime('24 hours ago');

     
    $request_stmt $db->prepare(
    SELECT registered 
    FROM requests
    WHERE email_address = :email_address AND timestamp >= :time
    '
    );
     
    $request_stmt->execute(array( 
            
    ':email_address' => $_POST['email'],
            
    ':time' => $time 
        
    )); 

    $times $request_stmt->rowCount();    
    var_dump($times);
    if ( 
    $times <= 10 )    
    {   
    echo 
    "ok";

    the table requests:
    Code:
    CREATE TABLE requests (
    email_address CHAR(64),
    timestamp DATETIME NOT NULL, 
    registered BOOLEAN
    ) ENGINE=InnoDB
    ;
  4. --
    Devshed Expert (3500 - 3999 posts)

    Join Date
    Jul 2012
    Posts
    3,957
    Rep Power
    1046
    So what's the problem?

    You should really start giving your tables sensible names. So I guess "requests" contains the emails you've sent? Then name it "sent_emails" or something like that.

    Don't do row counting in PHP. It's horribly inefficient, because it fetches all rows, counts them and then throws them away. Use COUNT(*).

    Last but not least, don't rely on the DateTime object to be magically converted to a MySQL timestamp. You need to actually construct a timestamp string in the proper format. The method for that happens to be called format().
    The 6 worst sins of security ē How to (properly) access a MySQL database with PHP

    Why canít I use certain words like "drop" as part of my Security Question answers?
    There are certain words used by hackers to try to gain access to systems and manipulate data; therefore, the following words are restricted: "select," "delete," "update," "insert," "drop" and "null".
  5. Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Feb 2013
    Posts
    392
    Rep Power
    8
    var_dump($times["COUNT(*)"]) gives "0" all the time... please help. I think the fault is at the comment lines in the code.


    PHP Code:
    $time = new DateTime('24 hours ago'); 
    $time_formatted $time->format('Y-m-d H:i:s'); 

    $request_stmt $db->prepare('  
    SELECT COUNT(*)  
    FROM sent_emails
    WHERE email_address = :email_address AND timestamp >= :time ///////// fault?
    '
    ); 
     
    $request_stmt->execute(array(  
            
    ':email_address' => $_POST['email'], 
            
    ':time' => $time_formatted  
        
    ));  

    $times $request_stmt->fetch(); 
    var_dump($times["COUNT(*)"]);
    if ( 
    $times["COUNT(*)"] <= 10 )     
    {    
    echo 
    "ok"




    $user_stmt $db->prepare(
            SELECT 
                1 
            FROM 
                users 
            WHERE 
                email = :email 
        '
    ); 
        
    $user_stmt->execute(array( 
            
    ':email' => $_POST['email'
        )); 
        
    $registered $user_stmt->fetchColumn(); 

    $request_stmt $db->prepare('  
    INSERT INTO sent_emails (
    email_address,
    timestamp,
    registered
    ) VALUES (
    :email_address,
    :timestamp,
    :registered
    )'
    ); 
     
    $request_stmt->execute(array(  
            
    ':email_address' => $_POST['email'], 
            
    ':timestamp' => $time_formatted,
            
    ':registered' => $registered  
        
    )); 
  6. No Profile Picture
    Contributing User
    Devshed Frequenter (2500 - 2999 posts)

    Join Date
    Dec 2004
    Posts
    2,969
    Rep Power
    374
    you should do:

    select count(*) as count from.......

    then further down:

    $times = $request_stmt->fetch();
    var_dump($times[count]); //
  7. Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Feb 2013
    Posts
    392
    Rep Power
    8
    ok thanks, but I'm still getting a 0...
  8. --
    Devshed Expert (3500 - 3999 posts)

    Join Date
    Jul 2012
    Posts
    3,957
    Rep Power
    1046
    Well, start debugging. The code itself has no errors, so it's obviously a problem of either the database or the input values.

    • How many rows do you have in the table?
    • What happens when you remove one or both of the conditions?
    • Get the input values and run the query manually.

    etc.

    By the way, I'm assuming your "/////" comment isn't in the actual code? Because that's not valid SQL syntax, and the question mark will also break the prepared statement.
    The 6 worst sins of security ē How to (properly) access a MySQL database with PHP

    Why canít I use certain words like "drop" as part of my Security Question answers?
    There are certain words used by hackers to try to gain access to systems and manipulate data; therefore, the following words are restricted: "select," "delete," "update," "insert," "drop" and "null".
  9. Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Feb 2013
    Posts
    392
    Rep Power
    8
    sorry, if you read this post before edited, it was wrong!


    Jacques, the fault was that I inserted the time that should be now as 24 hours ago...
    Last edited by derplumo; May 13th, 2013 at 02:53 PM.
  10. --
    Devshed Expert (3500 - 3999 posts)

    Join Date
    Jul 2012
    Posts
    3,957
    Rep Power
    1046
    Originally Posted by derplumo
    The query controles the time (later = ok) but not the date. How could I solve this?
    ??

    Just because you've named the parameter "time" doesn't make it a time. It's a timestamp. You've written it down yourself a few lines earlier:

    PHP Code:
    $time_formatted $time->format('Y-m-d H:i:s'); 
    That's a timestamp consisting of the year, the month, the day, the hours, the minutes and the seconds.

    Like I said: The code has no errors. I've checked it, and I've tested it. So please go through the debugging steps I've given you. You should also rename the parameter, since it's obviously confusing.
    The 6 worst sins of security ē How to (properly) access a MySQL database with PHP

    Why canít I use certain words like "drop" as part of my Security Question answers?
    There are certain words used by hackers to try to gain access to systems and manipulate data; therefore, the following words are restricted: "select," "delete," "update," "insert," "drop" and "null".
  11. Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Feb 2013
    Posts
    392
    Rep Power
    8
    I think we're done now with the secure login with password recovery... aren't we?

    the piece of code is now:
    PHP Code:
    $time = new DateTime('24 hours ago');  
    $time_formatted $time->format('Y-m-d H:i:s');  

    $request_stmt $db->prepare('   
    SELECT COUNT(*) as count  
    FROM sent_emails 
    WHERE email_address = :email_address AND timestamp >= :time 
    '
    );  
     
    $request_stmt->execute(array(   
            
    ':email_address' => $_POST['email'],  
            
    ':time' => $time_formatted   
        
    ));   

    $times $request_stmt->fetch();  
    if ( 
    $times['count'] <= 10 )      
    {     
    $user_stmt $db->prepare('  
            SELECT  
                1  
            FROM  
                users  
            WHERE  
                email = :email  
        '
    );  
        
    $user_stmt->execute(array(  
            
    ':email' => $_POST['email']  
        ));  
        
    $registered $user_stmt->fetchColumn();  

    $request_stmt $db->prepare('   
    INSERT INTO sent_emails ( 
    email_address, 
    timestamp, 
    registered 
    ) VALUES ( 
    :email_address, 
    NOW(), 
    :registered 
    )'
    );  
     
    $request_stmt->execute(array(   
            
    ':email_address' => $_POST['email'],   
            
    ':registered' => $registered   
        
    ));  
        
        
    // rest of the code in forgot password 
  12. --
    Devshed Expert (3500 - 3999 posts)

    Join Date
    Jul 2012
    Posts
    3,957
    Rep Power
    1046
    Wait, that makes no sense.

    Unregistered email addresses are supposed to only get a single email every 24 hours (see #240). Why should they be reminded 10 times that they're not registered on this site? I'm sure they're not that forgetful.

    The 10 mails limit refers to registered users. They should be able to request multiple emails, but not more than 10, because that would enable attackers to flood them with emails.

    Also, you cannot add the address to the "sent_emails" table at this point, because you don't even know whether you'll send an email. If the address is blocked, then you won't. And why do you store the emails as plaintext? I suggested using hashes. I also don't see the purpose of the "registered" field. You need two fields: "email" containing the SHA-256 hash of the email address. And "sent_timestamp" containing a DATETIME.

    Go back to the diagram in #234: Before the "generate token", you check whether the limit for registered addresses has been reached (10 in 24 hours). And before the "send email", you check whether the limit for unregistered addresses has been reached (1 in 24 hours). If it has not been reached, you send the corresponding email. And if that was successful (you need to check the return value of your mail function), then you can insert the address into the "sent_emails" table.

    Note: You must not display an error message in case the mail limit has been reached, because this again would allow attackers to check the existence of addresses (if they can make more than one request in 24 hours, the address exists).
    Last edited by Jacques1; May 14th, 2013 at 06:58 AM.
    The 6 worst sins of security ē How to (properly) access a MySQL database with PHP

    Why canít I use certain words like "drop" as part of my Security Question answers?
    There are certain words used by hackers to try to gain access to systems and manipulate data; therefore, the following words are restricted: "select," "delete," "update," "insert," "drop" and "null".
  13. Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Feb 2013
    Posts
    392
    Rep Power
    8
    I updated the script, I will post it tomorrow. Thanks for the critics!
  14. Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Feb 2013
    Posts
    392
    Rep Power
    8
    forgot_password.php:

    PHP Code:
    <?php 

       
        
    require("common.php"); 
        require(
    "lib/rnum.php");
        require(
    "lib/mail.php");
        require(
    "lib/password.php"); 

       
        
    if(!empty(
    $_POST)) 

     if(
    filter_var($_POST['email'], FILTER_VALIDATE_EMAIL)) 
     {    

    $hash hash('sha256'$_POST['email']);
    $time = new DateTime('24 hours ago'); 
    $time_formatted $time->format('Y-m-d H:i:s'); 

    $count_stmt $db->prepare('  
    SELECT COUNT(*)  as count
    FROM sent_emails
    WHERE email_address = :email_address AND timestamp >= :time 
    '
    ); 
     
    $count_stmt->execute(array(  
            
    ':email_address' => $hash
            
    ':time' => $time_formatted  
        
    ));  

        
    $times $count_stmt->fetch(); 
         
    $email $_POST['email'];
        
    $user_stmt $db->prepare(
            SELECT 
                id 
            FROM 
                users 
            WHERE 
                email = :email 
        '
    ); 
        
    $user_stmt->execute(array( 
            
    ':email' => $_POST['email'
        )); 
        
    $user_id $user_stmt->fetchColumn(); 
        if(
    $user_id)
        {       
        if(
    $times['count'] < 10)
        {
            
    $deactivation_stmt $db->prepare(
                UPDATE 
                    responses 
                SET 
                    active = 0 
                WHERE 
                    user = :user 
            '
    ); 
            
    $deactivation_stmt->execute(array( 
                
    ':user' => $user_id 
            
    )); 
                    
            
    $password_token rnum();
     
             
    $query 
            INSERT INTO responses ( 
            reset_key,
            user,
            secret,
            request_timestamp,
            request_ip
            ) VALUES ( 
            :reset_key,
            :user,
            :secret,
            NOW(),
            :request_ip
            )"
    ;
     
            
    $secret password_hash($password_tokenPASSWORD_BCRYPT, array("cost" => 10));  
            
    $reset_key rnum();
            
    $user $user_id['id'];
            
    $request_ip getenv('REMOTE_ADDR');

            
    $query_params = array( 
                
    ':reset_key' => $reset_key,
                
    ':user' => $user,
                
    ':secret' => $secret,
                
    ':request_ip' => $request_ip
            
    ); 
                    
            
    $stmt $db->prepare($query); 
            
    $result $stmt->execute($query_params);                 
            
            
    $url 'http://www.yourdomain.com/response_forgot_password.php?reset_key=';
            
    $url .= $reset_key;
            
    $url .= '&user=';
            
    $url .= $user;
            
    $url .= '&password_token=';
            
    $url .= $password_token;

            
    $mail_to      $email;
            
    $mail_subject 'Forgot password';
            
    $mail_body 'Click this link to reset your password:<br><br>';
            
    $mail_body .= $url;

            
            if(
    mail_f ($mail_to$mail_subject$mail_body) == 1)
            {
            
    $new_stmt $db->prepare('  
    INSERT INTO sent_emails (
    email_address,
    timestamp
    ) VALUES (
    :email_address,
    NOW()
    )'
    ); 
     
    $new_stmt->execute(array(  
            
    ':email_address' => $hash        
        
    ));       
            echo 
    'An email with further instruction has been sent to ' htmlentities($_POST['email'], ENT_QUOTESENT_HTML401'UTF-8'); 
            }
            else
            {
            echo 
    "Sorry, there was a technical issue. Please try again.";
            }
            }
            }
            else{
                if(
    $times['count'] < 1)
                {
                
    $email_adress $_POST["email"];  
                
    $hash hash('sha256'$email_adress);
                
    $user_stmt $db->prepare(
                    SELECT
                        1
                    FROM
                        unsubscribed_email_addresses
                    WHERE
                        email_address = :hash
                '
    ); 
                
    $user_stmt->execute(array( 
                    
    ':hash' => $hash
                
    )); 
                
    $user_sub $user_stmt->fetchColumn();
                if(
    $user_sub)
                {
                
    $user_stmt $db->prepare(
                    SELECT
                        unsubscribed
                    FROM
                        unsubscribed_email_addresses
                    WHERE
                        email_address = :hash
                '
    ); 
                
    $user_stmt->execute(array( 
                    
    ':hash' => $hash
                
    )); 
                
    $user_sub $user_stmt->fetchColumn();
                    if(
    $user_sub['unsubscribed'] != 1)
                    {
                    
    $email_key rnum();
                    
    $email_hash hash('sha256'$email_adress);
                                                
                    
    $url_2 'http://www.yourdomain.com/unsubscribe.php?email_key=';
                    
    $url_2 .= $email_key;
                    
    $url_2 .= '&email_address=';
                    
    $url_2 .= $email_hash;
                
                    
    $mail_to      $email;
                    
    $mail_subject 'Forgot password';
                    
    $mail_body .= "<br><br> If you don't wish to receive any e-mails from us again, please click this link:<br><br>";
                    
    $mail_body .= $url_2;
                    
                    if(
    mail_f ($mail_to$mail_subject$mail_body) == 1)
                    {
                    
    $new_stmt $db->prepare('  
    INSERT INTO sent_emails (
    email_address,
    timestamp
    ) VALUES (
    :email_address,
    NOW()
    )'
    ); 
     
    $new_stmt->execute(array(  
            
    ':email_address' => $hash        
        
    ));  

    }
    else {
    echo 
    "Sorry, there was a technical issue. Please try again.";
    }
                    }
                }
                else {
                
    $email_key rnum();
                
    $email_hash hash('sha256'$email_adress);
                
                
    $unsubscribe_stmt $db->prepare(
                    INSERT INTO unsubscribed_email_addresses ( 
                email_key,
                email_address
                ) VALUES ( 
                :email_key,
                :email_address
                )
                '
    ); 
                
    $unsubscribe_stmt->execute(array( 
                    
    ':email_key' => $email_key,
                    
    ':email_address' => $email_hash
                
    )); 
                        
                
    $url_2 'http://www.yourdomain.com/unsubscribe.php?email_key=';
                
    $url_2 .= $email_key;
                
    $url_2 .= '&email_address=';
                
    $url_2 .= $email_hash;
                
                
    $mail_to      $email;
                
    $mail_subject 'Forgot password';
                
    $mail_body .= "<br><br> If you don't wish to receive any e-mails from us again, please click this link:<br><br>";
                
    $mail_body .= $url_2;
                
                if(
    mail_f ($mail_to$mail_subject$mail_body) == 1)
                {
                
    $new_stmt $db->prepare('  
    INSERT INTO sent_emails (
    email_address,
    timestamp,
    ) VALUES (
    :email_address,
    NOW()
    )'
    ); 
     
    $new_stmt->execute(array(  
            
    ':email_address' => $hash,  
        ));  

    }
    else {
    echo 
    "Sorry, there was a technical issue. Please try again.";
    }
                }
                }
            }
     }
     else{
      echo 
    "This emailadress is invalid!";
     }               
    }
    ?> <html>
    <body>
    <form action="forgot_password.php" method="post"> 
        Email:
        <input type="text" name="email" value="" /> 
        <br /><br /> 
        <input type="submit" value="Recover" /> 
    </form> 
    </body>
    </html>
  15. --
    Devshed Expert (3500 - 3999 posts)

    Join Date
    Jul 2012
    Posts
    3,957
    Rep Power
    1046
    The overall logic looks good.

    There are only three issues left:

    (1)
    You mustn't give specific feedback about whether you've sent a mail or ran into an error etc. This allows me to check the existence of email addresses: If I make two requests and get a reaction after the second one, I know the address is registered, because unregistered addresses are already blocked after the first request. Remove all echos except the one about the invalid address. Then at the end of the "if valid address" block (between line 240 and 241) put a general message saying something like "Unless your limit has been reached, we'll send you an email". Check the refactored code below. Yes, this message will be a lie if your mailer is broken. If you want error-resistance, you'll need a queue approach*.

    (2)
    Your unsubscription tokens for addresses that already exist will not work. You generate a new token, but you don't insert it into the database. This way it won't be valid when the user tries to use it. Not inserting the address again is actually correct, but you either need to update the token or reuse the old one. I suggest the second variant: Simply send the old token again. There's no reason to regenerate it.

    You can also remove some duplicate code by refactoring this part (line 131Ė238):

    PHP Code:
    <?php

    # the following is for an unregistered address that hasn't reached its request limit yet

    # you only need one query
    $unsub_data_stmt $db->prepare('  
        SELECT 
            unsubscribed
            , email_key
        FROM 
            unsubscribed_email_addresses 
        WHERE
            email_address = :hash
    '
    );
    $unsub_data_stmt->execute(array(  
        
    ':hash' => $hash 
    ));  
    $unsub_data $address_unsub_data_stmt->fetchColumn();

    // If we don't have a record of the address yet, or if the address isn't unsubscribed,
    // send an email; in case of a new record, generate a new token, otherwise, use the old one;
    // $valid_token determines whether the newly generated token has been stored and can actually
    // be used; if not, it shouldn't be in the mail
    $send_mail $valid_token false;
    if ( 
    $unsub_data === false )
    {
        
    $send_mail true;
        
    $unsub_token rnum();
        
    $unsubscribe_stmt $db->prepare('  
            INSERT INTO unsubscribed_email_addresses (  
                email_address
                , email_key
            ) VALUES (  
                :email_key
                , :email_address 
            ) 
        '
    );  
        
    $valid_token $unsubscribe_stmt->execute(array(
            
    ':email_address' => $email_hash
            
    ':email_key' => $unsub_token
        
    ));
    }
    elseif ( !
    $unsub_data['unsubscribed'] )
    {
        
    $send_mail $valid_token true;
        
    $unsub_token $unsub_data['email_key'];
    }

    if ( 
    $send_mail )
    {
        
    # put the mail text into an external template; only append the token if $valid_token
        
    if( mail_f($email_adress$mail_subject$mail_body) ) 
        { 
            
    $sent_stmt $db->prepare('   
                INSERT INTO sent_emails (
                    email_address, 
                    , timestamp 
                ) VALUES ( 
                    :email_address
                    , NOW() 
                )
            '
    );  
            
    $sent_stmt->execute(array(   
                
    ':email_address' => $hash         
            
    ));
        }
    }
    (3)
    You should rephrase the email. The purpose of the mail is to inform the user about the unregistered address, so that should be in the text. The unsubscription is only a secondary feature.

    Last but not least, you should fix the indentation, add some comments and replace all the "user_stmt" with sensible names. I think that's it.



    * When a user makes a requests, you only save the request in the database. The actual processing (generating a token, sending the email etc.) is done by a cron job in a background. This way you'll never lose a request, even if the mailer is temporarily down. You also have a security benefit, because you could completely isolate the addresses from your application. This would make them much harder to steal. However, it's kind of an avanced feature and not necessarily a must-have. If you're interested, you should open a new thread.
    The 6 worst sins of security ē How to (properly) access a MySQL database with PHP

    Why canít I use certain words like "drop" as part of my Security Question answers?
    There are certain words used by hackers to try to gain access to systems and manipulate data; therefore, the following words are restricted: "select," "delete," "update," "insert," "drop" and "null".

IMN logo majestic logo threadwatch logo seochat tools logo