4
\$\begingroup\$

I am recently making a CMS and I need a secure login system, so this is my code. How is it?

First: the HTMLcode for signup and login:

<div id="login" style="display: none">
    <input type="text" name="email" id="login-email" placeholder=""><br><br>
    <input type="password" name="password" id="login-password" placeholder=""><br><br>
    <button type="button" id="submit-lodin"></button>
</div>
<div id="signup" style="display: none">
    <input type="text" name="firstname" id="firstname" placeholder=""><br><br>
    <input type="text" name="lastname" id="lastname" placeholder=""><br><br>
    <input type="text" name="email" id="email" placeholder=""><br><br>
    <input type="password" name="password" id="password" placeholder=""><br><br>
    <input type="password" name="re_password" id="re_password" placeholder=""><br><br>
    <button type="button" id="submit-signup"></button>
</div>

Second, the code for jQuery Ajax request for signup:

$(function(){
    $("#submit-signup").click(function(){

    var firstname   = $("#firstname").val();
    var lastname    = $("#lastname").val();
    var email       = $("#email").val();
    var password    = $("#password").val();
    var re_password = $("#re_password").val();


    if(password === re_password){
        if(validateEmail(email)){

            if(firstname == "" || lastname == "" || email == "" || password == ""|| re_password == ""){
                alert("");
            }else{
                $.ajax({
                    url : "resourcs/check_email.php",
                    type: "POST",
                    data : "firstname="+firstname+"&lastname="+lastname+"&email="+email+"&password="+password+"&re_password="+re_password  ,
                    success : function(d)
                    {
                        if(d === "ok"){
                            $.ajax({
                                url : "resourcs/register.php",
                                type: "POST",
                                data : "firstname="+firstname+"&lastname="+lastname+"&email="+email+"&password="+password+"&re_password="+re_password  ,
                                success : function(d2)
                                {
                                    if(d2 === "ok"){
                                                                                    location.reload(true);
                                    }else{
                                    alert(d2);
                                    }
                                }
                            });                          
                        }else{
                            switch(d){
                                case "error1":
                                    alert("");
                                break;                                    
                                case "error2":
                                    alert("");
                                break;                                    
                                case "error3":
                                    alert("");
                                break;                                    
                                case "error4":
                                    alert("");
                                break;    
                            };

                        }
                    }
                });     



            }
        }else{
        alert("");
        }
    }else{
    alert("");
    }

});

});
function validateEmail(email) { 
    var re = /^(([^<>()[\]\\.,;:\s@\"]+(\.[^<>()[\]\\.,;:\s@\"]+)*)|(\".+\"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/;
    return re.test(email);
}

My jQuery Ajax code for login:

$(function(){
 $("#submit-lodin").click(function(){
    var email = $("#login-email").val();
    var password = $("#login-password").val();
    if(email != "" && password != ""){
        $.ajax("resourcs/login.php",{
            type : "POST",
            data : "email="+email+"&password="+password,
            success : function(data){
                if(data === "ok"){
                                        location.reload(true);
                }else{
                    alert(data);
                }
            }
        });
    }
 });
});

My php code for login:

session_start();
include_once '../includes/config.php';


if(isset($_POST['email']) && isset($_POST['password'])&& $_POST['password'] != ""){
    $email = mysql_real_escape_string(strip_tags($_POST['email']),$connect);
    $password = mysql_real_escape_string(strip_tags($_POST['password']),$connect);
    $sql   = "select * from users where u_email = '$email'";
    $query = mysql_query($sql) or die(mysql_error());
    if(mysql_num_rows($query) > 0){
        while ($row  = mysql_fetch_object($query)) {
            $salt    = $row->u_salt;
            $grade = $row->u_grade;
            $id = $row->u_id;
        }

        $n_password  = md5(md5($password).$salt);
        $sql2 = "SELECT *
                FROM users
                WHERE u_email = '$email'
                AND u_password = '$n_password'";
        $query2=  mysql_query($sql2)  or die(mysql_error());
        if (mysql_num_rows($query2) == 1){
                $_SESSION ['login'] = "login";
                $_SESSION ['user_id'] = $id;
                $_SESSION ['email'] = $email;
                $_SESSION ['grade'] = $grade;
                echo"ok";

        }else{
            echo '';
        }

    }  else {
        echo '';
    }

}

My php code for signup is:

session_start();
include_once '../includes/config.php';
    $salt = '';
    for ($i = 0; $i < 3; $i++)
    {
        $salt .= chr(rand(33, 126));
    }
    $email       =  mysql_real_escape_string(strip_tags($_POST["email"]),$connect);
    $first_name  =  mysql_real_escape_string(strip_tags($_POST["firstname"]),$connect);
    $last_name   =  mysql_real_escape_string(strip_tags($_POST["lastname"]),$connect);
    $password    =  mysql_real_escape_string(strip_tags($_POST["password"]),$connect);
    $re_password =  mysql_real_escape_string(strip_tags($_POST["re_password"]),$connect);

    $password_e    =  md5(md5($password).$salt);
    $sql="INSERT INTO `users` (
            `u_firstname` ,
            `u_lastname` ,
            `u_password` ,
            `u_email` ,
            `u_salt`
            )
            VALUES (
            '".$first_name."',"
            . " '".$last_name."',"
            . "'".$password_e."', "
            . "'".$email."',"
            . " '".$salt."'
            )";
    $query=mysql_query($sql) or die(mysql_error());
    if ($query){
        $sql2 ="select * from users where u_email = '$email'";
        $query2=  mysql_query($sql2);
        while ($row = mysql_fetch_object($query2)) {
            $id=$row->u_id;
        }
        $_SESSION['login']="login";
        $_SESSION['user_id']=$id;
        $_SESSION['email']=$email;
        $_SESSION['isadmin']=2;
        echo 'ok';

    }

Finally, my database structure is:

CREATE TABLE IF NOT EXISTS `users` (
`u_id` int(11) NOT NULL,
  `u_firstname` varchar(255) CHARACTER SET utf8 NOT NULL,
  `u_lastname` varchar(255) CHARACTER SET utf8 NOT NULL,
  `u_password` varchar(255) COLLATE utf8_unicode_ci NOT NULL,
  `u_email` varchar(255) COLLATE utf8_unicode_ci NOT NULL,
  `u_grade` int(1) NOT NULL DEFAULT '1',
  `u_salt` varchar(255) COLLATE utf8_unicode_ci NOT NULL
) ENGINE=InnoDB AUTO_INCREMENT=4 DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;

Are there any security flaws in this code?

\$\endgroup\$
4
\$\begingroup\$

Flow Security

  • You should repeat the email validation in Server side too (js validation could be bypassed)
  • You don't check if the user is already inserted during sign up. There is no unique key in your users table. So I can signup with your admin email and (according with your code) I'm in. If somewhere in your code you don't check the users.id but email only, then you have a problem.
  • You should not login the client after the sign up, expecially if you don't force an email check (validation link). Generally this is a possible backdoor if you have some other bug somewhere (as first point, for example). Another point is to allow bots to entry in restricted area. That's not a great idea.

Structure security

  • use unique key on fields that must be unique (email in this case)

Language security

--

data : "email="+email+"&password="+password,

With a real data will be:

data : "email=my@email.it&password=my&!verystrong!!_#@[**password,

So my very strong password will be truncated at 2nd char. Both login and signup have the same issue. So my password will be valid and never I can suspect this issue. But a bruteforce attack will be more effective (my password will be simply "my")

\$\endgroup\$
  • \$\begingroup\$ " you don't check the users.id but email only" what do you mean \$\endgroup\$ – tom fox Feb 7 '15 at 17:16
  • \$\begingroup\$ If somewhere in your code you trust only the email stored in the session, then this is a possible backdoor. (I'm logged in with an email never checked) \$\endgroup\$ – Luca Rainone Feb 7 '15 at 17:19
  • \$\begingroup\$ so now what i need to secure my code is generate an access token during the login process and save it in session then hash it and save it in database and when login i check if this token after hash it match that in the database 2- is to make unique email and modify the JavaScript code to be use json instead of this method the change all the requests to the database from MySQL to PDO so after all these change my login now is secure 3-send validation messages to validate the email during register \$\endgroup\$ – tom fox Feb 7 '15 at 17:26
  • \$\begingroup\$ @tomfox "secure my code" is too generic :-). There are several levels of "security". I have just report what is not secure in your code already written (as suggested by bob hound). There are other things that could have sense to insert in your script. It depends what level of security do you need. (links of bob's answer are a point to start) \$\endgroup\$ – Luca Rainone Feb 8 '15 at 19:09
2
\$\begingroup\$

To answer your question, no, your login system isn't secure. chumkiu's answer just covers the obvious issues. It'll take more effort than it is worth to actually secure it. You are better off using a pre-built login system that has been hardened against multiple attack vectors. Here are a couple to choose from:

Rolls-Royce: http://barebonescms.com/documentation/sso/

Pinto: http://www.php-login.net/

The former has everything you will ever need in a login system. The latter is bizarrely split off into multiple projects, which makes maintenance for the project maintainers a lot harder, but their software at least aims to be secure and is backed with experience.

\$\endgroup\$
  • \$\begingroup\$ i can't configure the php-login in localhost to send validation messages to the registered email \$\endgroup\$ – tom fox Feb 8 '15 at 19:41
2
\$\begingroup\$

I hate to break it to you, but no this is not very secure. While you are using some techniques to help secure it, there are still a few flaws with how you are managing the input and SQL checks.

mySQL vs. mySQLi

USE MYSQLI... you need the "i", mySQL is old and deprecated and should not be used if you are using a new enough version of PHP. 7+ Supports the use of mysqli. If you're still running PHP 5.6 or older, you need to get that upgraded, seriously drop everything and focus on that. There are serious security vulnerabilities and to still be using it in 2019 is disgraceful to the security of your users.

Edit: I read this part back, and I don't mean to convey this as an insult, its just something that can be overlooked. If you are able to fix it, it should be fixed ASAP. If you can't, you should at least try to figure out who can.

Prepared Statements

After that, the first thing I should mention is prepared statements, these help you get rid of that darn mysqli_real_escape_string which is verbose and tedious to type over and over again. Instead you should be using what is called a prepared statement (even when you don't need too, its still a good habit). This negates the use of mysqli_real_escape, since the statement and parameters are sent separately and it is simply not possible to execute any SQL function via an input using the method below. You can also add in a preg_match($regex, $var) if you really want to get picky with inputs and confirming they are valid before insertion.

$email = input_filter(INPUT_POST, 'email', FILTER_VALIDATE_EMAIL);

if(!email){
// ERROR
}else{
  $sql = "SELECT u_email FROM users WHERE u_email = ?";
  $stmt = $con -> prepare($sql);
  $stmt -> bind_param("s", $email)
  $stmt -> execute();
}

After you start to get comfortable with this type of PHP mySQLi I suggest looking into this function from PHP Delusions. It has been an extremely handy tool for automating part of the preparation and execution, leaving you more time to deal with the results. As with what I did, do not fall into the trap of using procedural syntax, just stick with the object oriented version, its much easier to understand and a lot less verbose.

Password

MD5 is not meant for passwords, even with Salt. Please use the password_hash and password_verify functions to handle passwords, those are the best option you have for encrypting passwords with PHP. See my answer here for a deeper explanation on how to use them.

My own SQL email check

NOTE: This is not part of registration, this is an account function to change their email once signed in. This is the most relevant example I have without rewriting a giant chunk of my own registration form. There is no point in copying and pasting this exact script as it likely will not work for this purpose, however the logic, usage of SQL prepared statements, and check for an already registered variable is all there. You should be able to tweak it to your purpose.

// SQL HELPER
function prepared_Query($con, $sql, $params, $types = ""){
    $types = $types ?: str_repeat("s", count($params));
    $stmt   = $con -> prepare($sql);
    $stmt -> bind_param($types, ...$params);
    $stmt -> execute();
    return $stmt;
}

$umail = filter_input(INPUT_POST, 'umail', FILTER_VALIDATE_EMAIL);

// SQL USER EMAIL
if(!empty($umail)){
    $sql = "SELECT user_mail FROM _MAIN_users WHERE user_mail = ?;";
    $stmt = prepared_Query($con, $sql, [$umail]);
    $stmt -> store_result();
    $num = $stmt -> num_rows();
    if($num > 0){
        $err[] = "Taken";
    }else{
        $stmt -> free_result();
        $stmt -> close();
        $sql = "UPDATE _MAIN_users SET user_mail = ? WHERE user_id = ?;";
        $stmt = prepared_Query($con, $sql, [$umail, $uid]);
        $err[] = "Updated";
    }
    $stmt -> close();
}else{
    $err[] = "No Change";
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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