Nothing happens when typing a wrong username. what's wrong in my code?

Question

I'm trying to understand what's went wrong with my code. it's a login proccess to cp. you do get the right outcome when writing the correct username (only username yet, just for that test). but you get nothing at all when you don't - nothing I've programmed it to do.

I'm REALLY desperate, tried to solve this the whole night. please help me. here's the code.

calculations:

if (isset($_POST['connection_made'])) { // is a connection been made? using a hidden input

$form_user = forms_filter($_POST['form_user']); // filter any tags or any unwanted actions first
$form_pass = forms_filter($_POST['form_pass']); // filter any tags or any unwanted actions first

if (strlen($form_user) > 5 AND strlen($form_user) < 16 AND strlen($form_pass) > 5 AND strlen($form_pass) < 16) {
        if (login_blank_filter($form_user, $form_pass) == true) { // are those values length shorter than the minimal value or longer? 

            $user_name = $form_user;
            $raw_password = $form_pass;
            $user_pass = password_hash($raw_password, PASSWORD_DEFAULT); // generating a hashed and salted password 

                $cp_validate_login = mysqli_query($data_connection, "SELECT * FROM `admins` WHERE `username` = '$user_name' ");
                if (!$cp_validate_login) { die('error: ' . mysql_error()); }

                while ($admin_row = mysqli_fetch_array($cp_validate_login)) {

                    if (mysqli_num_rows($cp_validate_login) == 1) {
                        echo "you made it.";
                        echo mysqli_num_rows($cp_validate_login);
                    }
                    else {
                        echo "not yet there.";
                        echo mysqli_num_rows($cp_validate_login);
                    }

                }
        }
        else {
            header('Location: index.php?login_status=2');
        }
}

}

Form:

        <?php 
    if (isset($_GET['login_status'])) {
        switch ($_GET['login_status']) {

            case 1:
            echo "wrong username or password";
            break;      

            case 2:
            echo "the inputs has to be filled";
            break;

            case 3:
            echo "username\passwords are too short or too long";
            break;

            default:
            echo "unknown error";
        }
    }
    else {
        echo "welcome. log in to the control panel please";
    }
    ?>
    </div>

    <form name="loginform" method="post" action="index.php" onSubmit="return validateBlank()">
    <input type="hidden" name="connection_made" value="">

    <div class="login_layout">

        <div class="login_right_layout">
        user:
        </div>

        <div class="login_left_layout">
        <input class="login_input" name="form_user" type="text">
        </div>

        <div class="login_right_layout">
        password:
        </div>

        <div class="login_left_layout">
        <input class="login_input" name="form_pass" type="password">
        </div>

    </div>

    <input type="submit" value="כניסה" class="login_submit">
    </form>


</center>

PLEASE help me. thank you.

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

0

First, you are wide open to SQL injection. You need to use prepared statements, rather than concatenating variables into your query. See How can I prevent SQL injection in PHP?.

Second, your code fails because you never make it into the while loop if the username is invalid. That is, if you type a bogus username, this condition is never satisfied:

while ($admin_row = mysqli_fetch_array($cp_validate_login))

So, your if/else logic is never executed.

The solution here is not to try to improve your existing code. It's to stop what you're doing and use an existing authentication (login) library. Security is hard, and proper authentication and authorization is no exception. You should not roll your own security code.

edited May 23 '17 at 12:32

Community

1
1

answered Mar 07 '17 at 02:25

elixenide

44,308
16
74
100

Firstly, many thanks. 1 - I'm glad you alerted me about that dangerous vars in the query. problem is with the link you gave me - I can't understand it, because it's examples are written by OOP - something I don't understand nor using. can yoo please give me something else? Also, thank you for the solution. should have figure it out myself (probably stayking awake 24 hours is not the best idea). About using somone else code - this is a one thing I never want to do. and once again, lack of OOP knowledge prevents me from working with those. Can I just "patch up" my current code? – Ori R Mar 07 '17 at 02:53
by the way, this is how I filtered all: function forms_filter($form_value) { return stripslashes(htmlspecialchars(strip_tags($form_value))); } – Ori R Mar 07 '17 at 03:46
@OriR Your `forms_filter()` function is not nearly enough to make it safe; the only safe option is prepared statements with parameters. If someone submits a username like `' OR 1=1 -- -`, your code will give that person admin access. As for using someone else's code: that absolutely *is* what you should do, *especially* with authentication, but *only* once you understand what the code does. I used to think the way you do, but that's very dangerous. If you just "patch up" your current code, you will inevitably miss other, major holes. Use a framework so you don't have to reinvent the wheel. – elixenide Mar 07 '17 at 04:32

Nothing happens when typing a wrong username. what's wrong in my code?

1 Answers1