How to resolve the problems with this login script?

Question

I am stuck on my coding: I need to check that the email is legitimate and also check if the passwords match.

As you can see I've tried to make sure it has '@' in it, but it doesn't work.

 <?php
    error_reporting(1);
    include("../site/inc/config.php");

    if (isset($_POST['submit'])) {
        $username = clean($_POST['username']);
        $password = clean($_POST['password']);
        $pass_conf = clean($_POST['password2']);
        $email = clean($_POST['email']);
        $ip = $_SERVER['REMOTE_ADDR'];
        $reg_date = date("F jS, Y - g:i a");
        $md5 = md5($password);

        if(!$username){
            header("Location: /site/register.php?error=1");
            //error
            die;
        }


        if(!$password){
            header("Location: /site/register.php?error=1");
            //error
            die;
        }


        if(!$pass_conf){
            header("Location: /site/register.php?error=1");
            //error
            die;
        }


        if(!$email){
            header("Location: /site/register.php?error=1");
            //error
            die;
        }

        $sign = '@';

        if (strpos($email,$sign) !== true) {
            header("Location: /site/register.php?error=122");
            //error
            die;
        }

        $check_user = mysql_query("SELECT `username` FROM `users` WHERE `username` = '".$username."'");

        if(mysql_num_rows($check_user) !== 0){
            header("Location: /site/register.php?error=2");
            //error
            die;
        }

        $check_email = mysql_query("SELECT `email` FROM `users` WHERE `email` = '".$email."'");

        if(mysql_num_rows($check_email) !== 0){
            header("Location: /site/register.php?error=3");
            //error
            die;
        }

        mysql_query("INSERT INTO `users` (`username`, `password`, `email`, `ip`, `rank`, `reg_date`) VALUES ('$username', '$md5', '$email', '$ip', '1', '$reg_date')");
        header("Location: /site/register.php?success=1");
    } else {
        header("Location: register.php?error=4");
    }

    ?>

score 1 · Answer 1 · edited May 23 '17 at 12:25

Ah. First of all, don't use deprecated mysql_ switch to mysqli.

Second, don't use $_POST, $_GET, $_COOKIE variables without checking whether they are set. Rather do:

$request_variables = array('password', 'password2', 'username', ...);
$data = array();
foreach( $request_variables as $k){
    if( !isset( $_POST[$k])){
        header( 'Location: http://...'); // Yes, use absolute url
        die();
    }

    $v = clean( $_POST[$k]);
    if( !$v){
        // header, die
    }

    $data[$k] = $v;
}

Then, make sure you're escaping data before pushing them into query. If not using prepared statements with parameters, at least use *sigh* mysql_real_escape_string and last but not least, for email validation just browse stack overflow for a while:

if (!filter_var($data['email'], FILTER_VALIDATE_EMAIL)) {
    header("Location: http://.../site/register.php?error=122");
    die();
}

Yeah, and password matching:

if( $data['password'] !== $data['password2']){
    header(...);
    die(...);
}

But I'd try to validate password match onsubmit of <form> too (so passwords would be sent just once, and also sent them both to server for case of disabled javascript).

score 0 · Accepted Answer · answered Apr 20 '13 at 19:53

0

if (strpos($email,$sign) === false) {
    /* error ! */
}

would be right, as strpos never returns true. It returns false or an integer. So an error is in this case if strpos is strictly equal to false.

For a password match, simply compare the passwords:

if ($password !== $pass_conf) {
    /* error ! */
}

P.s.: surround the $variables in queries by mysql_real_escape_string($variable). If not you risk SQL injections.

answered Apr 20 '13 at 19:53

bwoebi

23,637
5
58
79

Yeah, haha. I think it worked thanks. And j.s would this work? function clean($str){ //clean a string and return it return mysql_real_escape_string(strip_tags(stripslashes($str))); //return a much cleaner string. } I've put it on my config which will go on all pages – Elizabeth Edge Apr 20 '13 at 20:00
@ElizabethEdge This works, but the `stripslashes` call is absolutely unnecessary. (When my answer was helpful, please accept it.) – bwoebi Apr 20 '13 at 20:04

How to resolve the problems with this login script?

2 Answers2