0

I am making php login function and I have come across a problem. In one part of the script I am testing whether all of the info is inserted in html form that is fed to the script via $_POST variable. And in one part, the script correctly evaluates whether only username is not entered or only password, and it correctly evaluates whether password is wrong BUT when I enter correct user/pass, it activates error "Username and password not entered". I can't figure it out. Is it possible that FLASE && FALSE equals TRUE?

---Edit---- Ok, I see now that I should included all of the relevant files in this question. So here they are:

index.php

<?php
session_start();
if (isset($_SESSION['login_message'])) {
    $message = $_SESSION['login_message'];
    unset($_SESSION['login_message']);
}
?>

<html>

<head>
<?php
require_once("include/head.php");
?>
</head>

<body>

<form action="auth/login.php" method="post">

<table>

<tr>

<td>
<img src="graphics/znak_hrz.png" alt="Znak HRZ" style="height: 200px; padding: 10px;">
</td>

<td>
    <table style="padding: 10px;">
            <tr>
                <td><?php if (isset($message)) {echo "<td>" . $message . "</td>";}?></td>
            </tr>

            <tr>
               <td>
               <label for="username">Username:</label>
               <input id="username" type="text" name="username" />
             </td>
            </tr>

            <tr>
              <td>
              <label for="password">Password:</label>
              <input id="password" type="password" name="password" />
              </td>
            </tr>

    <tr>
            <td style="text-align: center;">
            <input type="submit" name="login" value="Login" />
            </td>
    </tr>

    </table>
</td>

<td>
<img src="graphics/znak_eskadrile.png" alt="Znak eskadrile" style="height: 200px; padding: 10px;">
</td>

</tr>

</table>

</form>

</body>

</html>

login.php

<?php
session_start();

// This script will deny access if following conditions are met in that order:
// - Username not entered
// - Password not entered
// - Username and password not entered
// - User doesn't exist in the database
// - User is deactivated in the database
// - The password is wrong
// Upon successful login, it will redirect user to secure/index.php and
// upon unsuccessful login it will return him to index.php for another try.

// If username is not set, set an error message
if (empty($_POST['username']) && !empty($_POST['password'])) {
    $_SESSION['login_message'] = "Username missing";
}

// If password is not set, set an error message
if (empty($_POST['password']) && !empty($_POST['username'])) {
    $_SESSION['login_message'] = "Password missing.";
}

//If username AND password are not set, set an error message
if (empty($_POST['username']) && empty($_POST['password'])) {
    $_SESSION['login_message'] = "Username and password empty.";
}

// Check if the username exists in the database and if the password is correct
if (!isset($_SESSION['login_message']) && !empty($_POST['username']) && !empty($_POST['password'])) {

    require_once("database.php");

    // Sanitize incoming username and password
    $username = filter_var($_POST['username'], FILTER_SANITIZE_STRING);
    $password = filter_var($_POST['password'], FILTER_SANITIZE_STRING);

    // Determine whether an account exists matching this username and password
    $stmt = $auth_db->prepare("SELECT uid, pid, password, access_category, last_log, active FROM " . TBL_USERS . " WHERE username = ?");

    // Bind the input parameters to the prepared statement
    $stmt->bind_param('s', $username);

    // Execute the query
    $stmt->execute();

    // Assign results of query to temporary variables
    $stmt->bind_result($uid, $pid, $db_password, $access_category, $last_log, $active);
    $stmt->fetch();

        // If user doesn't exist in the database, deny login
        if (!isset($uid)) {
            $_SESSION['login_message'] = "User doesn't exist.";
        }

        // If user is deactivated, deny login
        if (isset($uid) && !$active) {
            $_SESSION['login_message'] = "User is deactivated.";
        }

        // If the password is wrong, deny login
        if (isset($uid) && $active && $db_password != md5($password)) {
            $_SESSION['login_message'] = "Wrong password.";
        }

        if (!isset($_SESSION['login_message'])) {

        // Close previous statement
        $stmt->close();

        // Update the account's last_login column
        $stmt = $auth_db->prepare("UPDATE " . TBL_USERS . " SET last_log = NOW() WHERE username = ?");
        var_dump($stmt);
        $stmt->bind_param('s', $username);
        $stmt->execute();

        // Set session variable
        $_SESSION['username'] = $username;
        $_SESSION['uid'] = $uid;
                $_SESSION['pid'] = $pid;
        $_SESSION['last_log'] = $last_log;
                $_SESSION['active'] = $active;
        $_SESSION['access_category'] = $access_category;
        }
}

if (!isset($_SESSION['login_message'])) {
    header('Location: ../secure/index.php');
} else if (isset($_SESSION['login_message'])) {
    header('Location: ../index.php');
}
?>

secure/index.php

<?php
    session_start();
    require_once("../auth/login.php");
?>

<html>

<head>
<?php
#if($_SESSION['access_category'] == '0') {
#   header('Location: eth93sl/');
#}
?>
</head>

<body>
<?php
    echo "uid:" . $_SESSION['uid'] . "<BR>";
    echo "username: " . $_SESSION['username'] . "<BR>";
    echo "active: " . $_SESSION['active'] . "<BR>";
    echo "last_log: " . $_SESSION['last_log'] . "<BR>";
    echo "access_category: " . $_SESSION['access_category'] . "<BR>";
?>
</body>

</html>
Fikus
  • 41
  • 7
  • Because query probably matches the value so it returns true, its how programming works... – Jack M. Aug 06 '14 at 14:22
  • 3
    It seems that you don't clear the $_SESSION['login_message'] – Maxim Krizhanovsky Aug 06 '14 at 14:23
  • 2
    Sidenote: If this is for a LIVE working site, I suggest you stop using MD5 as a password storage method; it's old (*circa 1992*) and considered broken. Use [**CRYPT_BLOWFISH**](http://security.stackexchange.com/q/36471) or PHP 5.5's [`password_hash()`](http://www.php.net/manual/en/function.password-hash.php) function. For PHP < 5.5 use the [`password_hash() compatibility pack`](https://github.com/ircmaxell/password_compat). – Funk Forty Niner Aug 06 '14 at 14:24
  • 2
    Also from a security standpoint, you should consider returning the exact same error message for the case where UserId does not exist and where User exists but password is wrong. Letting the person at the keyboard know just gives malicious people more information if they are trying to break into your system. They can try usernames all day and discover what user names are valid, for instance. – Jeff Aug 06 '14 at 14:34
  • Darhazer, I clear the $_SESSION['login_message'] in a different file, the index.php. This is login.php which gets called by form on index.php. Good point, but checked it already. – Fikus Aug 06 '14 at 14:38
  • Fred, thank you for the note, will implement it, although this is an application for accessing statistical info on a closed LAN so it is not that critical. Thank you all the same, it is a good practice that I will try to implement in my work. – Fikus Aug 06 '14 at 14:39
  • Also Jeff, I understand that, but since I won't be around to fix things so often on this closed network, I need as much feedback as I can get to identify the problem over the phone. And security is not such a big issue since access to the physical computers is limited. But good point. – Fikus Aug 06 '14 at 14:41
  • You say the output is "Username and password not entered" and I don't see that in your code. Try var_dump($_POST) before the if stuff to double check those variables are set. – Boris Aug 06 '14 at 14:42
  • Mr Jack, I understand how programming works, and it would make sense if I didn't enter username and password, but I have entered the correct ones, and matched conditional was supposed to filter out cases where neither username or password was entered. The question that bothers me is why, I have a feeling that I am missing something and was hoping to get another fresh opinion as to why could this be. – Fikus Aug 06 '14 at 14:48
  • Darhazer, here is the code from index.php where I clear $_SESSION variable and array: `session_start(); if (isset($_SESSION['login_message'])) { $message = $_SESSION['login_message']; unset($_SESSION['login_message']); } session_destroy();` – Fikus Aug 06 '14 at 14:50
  • @Boris, I meant to write "Username and password empty.", I appologize. I tried var_dump on $_POST['username'] and $_POST['password'] and returned values just before the evaluation in question are correct ones, just as I entered. The funny thing is if I enter wrong password, the correct evaluation triggers (which is below the evaluation of "username and password empty") which doesn't make sense to me. It seems as if the whole script runs twice for some reason. – Fikus Aug 06 '14 at 15:04
  • This SO answer has plenty of pointers for your problem http://stackoverflow.com/questions/10097887/using-sessions-session-variables-in-a-php-login-script – Adrian Aug 06 '14 at 15:07
  • Got it! I overlooked line three in secure/index.php. Because of that line the script was actually executing twice, and failing the second time! It is a remnant from another login system I was experimenting with... I was too focused on login.php to notice secure/index.php. Well, this was fun. Thank you guys, and @Adrian, good post, has some interesting info, thank you! – Fikus Aug 06 '14 at 17:07
  • @Fikus I suggest adding an answers to your own question in this case – Adrian Aug 07 '14 at 08:48

1 Answers1

0

The problem was that the login.php script was executing twice because of the line three in secure/index.php, a remnant line from another login system I was experimenting with. And second time that the script got called, it had no $_POST data, hence no username and no password so the apropriate conditional got activated.

Reminded me that when I come across a problem, it is always good to broaden my view to other files as well.

Fikus
  • 41
  • 7