4
\$\begingroup\$

I've created an admin login area for an application I am planning to code, and I've used the following login.html page to let the user type in his data (I left out parts like "id", "placeholder" etc. to make it shorter):

<form method="post" action="login.php">
  <input type="text" name="username">
  <input type="password" name="password">
  <input type="submit" value="Login" name="submit">
</form>

The form is processed as is, no JavaScript interfering here. The login.php then looks like this:

<?php
  if (isset($_POST['password']) && isset($_POST['userName'])) {
    if ($_POST['password'] == "myPassword" && $_POST['userName'] == "myUsername") {
      if (!session_id()) {
        session_start();
        $_SESSION['logon'] = true;
        header('Location: admin_area.php');
        die();
      }
    } else if ($_POST["password"] == "" || $_POST["userName"] == "") {
      echo "Please enter both a username and a password! You are sent back in 3 seconds";
      echo "<meta http-equiv='refresh' content='3;url=login.html'>";
    } else {
      echo "Wrong username and/or password! You are set back in 3 seconds";
      echo "<meta http-equiv='refresh' content='3;url=login.html'>";
    }
  } else {
    echo "This is a restricted area, you have to log in!";
    echo "<meta http-equiv='refresh' content='3;url=login.html'>";
  }
?>

The admin_area.php file, which the user is then being redirected to, has the following short placeholder code:

<?php
  if (!session_id()) session_start();
  if (!$_SESSION['logon']) { 
    echo "This is a restricted area, you have to log in!";
    echo "<meta http-equiv='refresh' content='3;url=login.html'>";
  } else {
    echo "<script src='//ajax.googleapis.com/ajax/libs/jquery/2.0.3/jquery.min.js'></script>";
    echo "<script type='text/javascript' src='session_destroy.js'></script>";
    echo "Hello World!";
    echo "<br>";
    echo "<input type='button' id='exit_button' value='Exit' onclick='window.open(\"\", \"_self\", \"\"); window.close();'>";
  }
?>

The essential part of the session_destroy.js file on this page contains this code snippet:

$(window).on("beforeunload", function() {
  $.get("session_destroy.php");
  return "You have left the page. Your session has been terminated.";
});

And finally, the session_destroy.php, which should terminate all session data, contains the following lines (taken from the PHP manual):

<?php
  if (!session_id()) session_start();
  if (!$_SESSION['logon']) { 
    echo "This is a restricted area, you have to log in!";
    echo "<meta http-equiv='refresh' content='3;url=login.html'>";
  } else {
    $_SESSION = array();
    if (ini_get("session.use_cookies")) {
      $params = session_get_cookie_params();
      setcookie(session_name(), '', time() - 42000,
        $params["path"], $params["domain"],
        $params["secure"], $params["httponly"]
      );
    }
    session_destroy();
    header('Location: login.html');
  }
?>

Now this "construct" seems to be working just fine. I am wondering if there are some improvements on these lines, as I have researched everything myself and I am not sure how "secure" and correct these methods might be.

I am thinking of possible security improvements to the form-processing and session-handling / destroying. Are there any areas in which I should do more research?

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Tha major security issue I see is that you do not have any limiting or logs for wrong/success attempts. One can use automated brute force attacks trying all day/night long to guess your username and password. If lack of success, at least you might have your app slowed down, because of the billion requests.

As you requested, I tried to add some code examples around my suggestion:

<?php

//the db interaction is pseudo code, because it depends on the API you are using
/**
 * 
 * @param int $success : 0 for fail, 1 for success
 */
function logAttempt($success) {
    $user = $db->escape($_POST['userName']);
    $pass = $db->escape($_POST['password']);
    $session_id = session_id();
    if (!in_array($success, array(0, 1))) {
        return false;
    }
    $db->query("INSERT INTO `log_attempts` 
                    (`user`, `pass`, `ip`, `session_id`, `success`, `tried_on`)
                VALUES
                    ('$user', '$pass', '{$_SERVER['REMOTE_ADDR']}', '$session_id', '$success', NOW());
            ");
    return $db->affected_rows > 0;
}

function isAttemptsLimitExceeded() {
    $max_attempts = 5;
    $session_id = session_id();
    $res = $db->query("SELECT 
                            COUNT(*) AS cnt
                        FROM 
                            `log_attempts`
                        WHERE
                            `session_id` = '$session_id' AND
                            `tried_on` > DATE_SUB(NOW(), INTERVAL 24 HOUR) AND
                            `success` = '0';
                    ");
    $row = $db->fetch($res);
    if ($row['cnt'] >= $max_attempts) {
        return true;
    }
    return false;
}

function banFailedLogins() {
    if(isAttemptsLimitExceeded()) {
        $db->query("INSERT INTO `blacklist`
                        (`ip`, `banned_on`)
                    VALUES
                        ('{$_SERVER['REMOTE_ADDR']}', NOW()");
    }
    return $db->affected_rows > 0;
}

function clearBans() {
    $db->query("DELETE from `blacklist` WHERE `ip` = '{$_SERVER['REMOTE_ADDR']}';");
    return $db->affected_rows > 0;
}

//check for expired bans?
function canAcess() {
    $ban_expire = 24; // hours
    $res = $db->query("SELECT 
                            COUNT(*) AS cnt 
                        FROM 
                            `blacklist` 
                        WHERE 
                            `ip` = '{$_SERVER['REMOTE_ADDR']}' AND
                            `banned_on` > DATE_SUB(NOW(), INTERVAL $ban_expire HOUR);
                        ");
    $row = $db->fetch($res);
    if ($row['cnt'] > 0) {
        return false;
    }
    else {
        clearBans();
        return true;
    }
}

if (!canAcess()) {
    die("You are banned for failed login attempts");
}
  if (isset($_POST['password']) && isset($_POST['userName'])) {
    if ($_POST['password'] == "myPassword" && $_POST['userName'] == "myUsername") {
      if (!session_id()) {
        session_start();
        $_SESSION['logon'] = true;
        logAttempt(1); //log successfull attempt
        header('Location: admin_area.php');
        die();
      }
    } else if ($_POST["password"] == "" || $_POST["userName"] == "") {
      echo "Please enter both a username and a password! You are sent back in 3 seconds";
      logAttempt(0); //log failed attempt
      banFailedLogins();
      echo "<meta http-equiv='refresh' content='3;url=login.html'>";
    } else {
      echo "Wrong username and/or password! You are set back in 3 seconds";
      logAttempt(0); //log failed attempt
      banFailedLogins();
      echo "<meta http-equiv='refresh' content='3;url=login.html'>";
    }
  } else {
    echo "This is a restricted area, you have to log in!";
    echo "<meta http-equiv='refresh' content='3;url=login.html'>";
  }

?>
\$\endgroup\$
5
  • \$\begingroup\$ Thanks for the great suggestion Royal Bg. May I ask, what sort of limitation for login attempts you suggest? Would a (clear) captcha login be enough to make sure that no bots hammer the login form, or do you recommend something else? Thanks again. \$\endgroup\$
    – tricon
    Dec 26, 2013 at 21:10
  • \$\begingroup\$ You can use database. Log the attempts there. If for certain session there are more than X attempts - add in a blacklist table and ban for Y hours. \$\endgroup\$
    – Royal Bg
    Dec 26, 2013 at 21:13
  • \$\begingroup\$ Do you maybe have a source for me, where I can check up and learn this technique? I get the basic Idea, I am just not that proficient with handling sessions (and as you mentioned "attempts for one certain session") yet. Thank you again. \$\endgroup\$
    – tricon
    Dec 26, 2013 at 21:28
  • \$\begingroup\$ session_id() is enough. You can insert everytime an user tries to login the session_id. Once there are 5 failed logins with th e same session_id - ban the user. You can change session_id() with IP for example. I added some code \$\endgroup\$
    – Royal Bg
    Dec 26, 2013 at 21:58
  • \$\begingroup\$ Thanks a lot for the explanation and the code Royal. This is a great help. I am going to translate your example into my pages tomorrow. \$\endgroup\$
    – tricon
    Dec 26, 2013 at 23:35

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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