5
\$\begingroup\$

After a lot of back and forth on various sites, reading articles, watching videos etc i still can not figure out the best way to secure my admin section.

The session id is regenerated every page reload / action to make session hijacking / fixation more difficult.

Let me know in the comments if you need any more details.

My Goal:

  • Securely log the user in
  • Set a session / cookie
  • Authenticate the user
  • Do this without HTTPS

The following code attempts to deal with authenticating the user / allowing use of the various admin pages / functions if successfully logged in, what am i missing?

Config.php

<?php 
session_name('wcx');
session_start();
session_regenerate_id(true);
define( 'DB_HOST', '' ); // set database host
define( 'DB_USER', '' ); // set database user
define( 'DB_PASS', '' ); // set database password
define( 'DB_NAME', '' ); // set database name

spl_autoload_register(function ($class) {
    require_once 'classes/class.'. $class .'.php';
});

$backend = new backend();

?>

Index.php

<?php
define('WCX', TRUE);
require_once('config.php');

$pagearray = array('dashboard');

if(isset($_COOKIE['wcxadmin'], $_SESSION['loggedin'], $_SESSION['session']) && $backend->isLoggedIn()===true) {
    if(isset($_GET['page']) && !empty($_GET['page'])) {
        $page = filter_input(INPUT_GET, 'page', FILTER_SANITIZE_STRING);
    }
    else {
        $page = 'dashboard';
    }

    include_once('includes/header.php');

    if(in_array($page, $pagearray, TRUE) && file_exists('includes/'.$page.'.php')) {
        include_once('includes/'.$page.'.php');
    }
    else {
        include_once('includes/404.php');
    }

    include_once('includes/footer.php');
}
else {
    include_once('login.php');
}
?>

Login.php

<?php
if(!defined('WCX')) {
   die('Direct access not permitted');
}
require_once("config.php");
?>

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8" />
<link href="css/login.css" rel="stylesheet" type="text/css" />
<title>WebCodex - Please Login</title>
</head>

<body>  
<div id="loginwrapper">
<h2>WCX LOGIN</h2> 
<div id="loginformwrap">
    <div id="loginformwrapper">    
        <form name="loginform" method="post" action="<?php $_SERVER['PHP_SELF']; ?>">
            <table class="adminlogin">
            <tr>
            <td valign="top">
            <input type="text" class="default" required="required" name="username"  value="" placeholder="Username" id="adminusername" maxlength="80" style="text-align: center; width:280px; color: #333;" onblur="this.style.color='#333'" onfocus="this.style.color='#000'"/> 
            </td>
            </tr>
            <tr>
            <td valign="top">
            <input type="password" class="default" required="required" name="password"  value="" placeholder="Password" id="adminpassword" maxlength="80" style="text-align: center; width:280px; color: #333;" onblur="this.style.color='#333'" onfocus="this.style.color='#000'"/> 
            </td>
            </tr>
            <tr>
            <td colspan="2">
            <input id="adminloginbutton" type="submit" name="login" value="LOGIN" />
            </td>
            </tr>
            </table>
        </form>

    </div>
    <?php 
    // SESSION ERRORS ARE SHOWN HERE
    if(isset($_SESSION['errors'])) {
        echo '<div class="loginerror">'.$_SESSION['errors'].'</div>';
        // CLEAR THE SESSION ERRORS AFTER DISPLAYING THEM
        $_SESSION['errors'] = '';
    }
     ?>
</div>
</div>

</body>
</html>
<?php
    // IF LOGIN BUTTON PRESSED / FORM SUBMITTED
    if (isset($_POST['login'])) {
        $bits = 32;
        $usercookie = 'Admin-'.bin2hex(openssl_random_pseudo_bytes($bits));

        // SET THE LASTLOGIN AND SESSION VARIABLES
        //$lastlogin = date('d / m / y - H:ia');
        $session = bin2hex(openssl_random_pseudo_bytes($bits));
        $active = 1;

        // GRAB THE USERS INPUT
        $username = $_POST['username'];
        $password = $_POST['password'];

        //SELECT PASSWORD WHERE USERNAME = $username using a Prepared Statement
        $query = 'SELECT password FROM wcx_admin WHERE username = :username';

        // PREPARE, BIND, EXECUTE, BIND THE RESULT TO A VAR $dbpass, FETCH, CLOSE
        $stmt = $backend->queryIt($query);
        $stmt = $backend->bind(':username', $username);
        $stmt = $backend->execute();
        $dbpass = $backend->getColumn();

            // VERIFY USER INPUTTED PASSWORD WITH DB PASSWORD USING PHP FUNCTION password_verify()
            if(password_verify($password, $dbpass)) {

            // UPDATE ACTIVE AND LASTLOGIN WHERE USERNAME = $username
            $query = 'UPDATE wcx_admin SET lastlogin = CURRENT_TIMESTAMP, session = :session WHERE username = :username';

            // PREPARE, BIND, EXECUTE, CLOSE
            $stmt = $backend->queryIt($query);
            $stmt = $backend->bind(':session', $session);
            $stmt = $backend->bind(':username', $username);

                // IF THE ABOVE WENT WELL
                if($backend->execute()) {

                    // SET THE SESSION 
                    $_SESSION['username'] = $backend->cipher($username, 1); 
                    $_SESSION['loggedin'] = 1;
                    $_SESSION['session'] = $session;

                    // A 1 HOUR LOGIN COOKIE
                    setcookie( 'wcxadmin', $usercookie, time()+3600);
                    header('Location: index.php');
                }
            }
                else {
                    // IF ERRORS SET AN ERROR SESSION w/GENERIC ERROR MESSAGE / DO NOT GIVE TOO MUCH AWAY
                    $_SESSION['errors'] = 'Incorrect Username or Password';
                    // REFRESH THE PAGE TO SHOW THE ERROR
                    header('Location: index.php');
                }
    }  
?>

isLoggedIn Function

// Is the User Logged In?
public function isLoggedIn() {
    if($_SESSION['loggedin'] === 1) {
        return true;
    }
    else {
        return false;
    }
}

Logged in check on every page

// Check for cookie and sessions / isLoggedIn boolean
if(isset($_COOKIE['wcxadmin'], $_SESSION['loggedin'], $_SESSION['session']) && $backend->isLoggedIn()===true) {
    //Do Something Here
}

Logout.php

<?php 
require_once('config.php');
if(isset($_COOKIE['wcxadmin'], $_SESSION['loggedin'], $_SESSION['session']) && $backend->isLoggedIn()===true) {
    $session = $_SESSION['session'];

    $query = 'UPDATE wcx_admin SET session = :sess WHERE session = :session ';
    $stmt = $backend->queryIt($query);
    $stmt = $backend->bind(':sess', $sess='');
    $stmt = $backend->bind(':session', $session);
    $logout = $backend->execute();

    if($logout) {
        $params = session_get_cookie_params();
        setcookie(session_name(), '', 0, $params['path'], $params['domain'], $params['secure'], isset($params['httponly']));
        setcookie('wcxadmin', '', time()-3600);
        session_unset();
        session_destroy();
        header('Location: index.php');
    }
}
else {
    header('Location: index.php');  
}
?>
\$\endgroup\$
3
\$\begingroup\$

Security

You have a lot of things covered. You use prepared statements and only include files that are defined in a white-list, that's good. I just have these smaller points:

  • don't store your password in the php source file, but in a configuration file (outside the web root). It's not too dangerous, but it's better to be save.
  • action="<?php $_SERVER['PHP_SELF']; ?>: this would be vulnerable to XSS attacks if it were working. As you are not echoing, the action will always be "". Either set the action to a hard-coded "", or echo an escaped php self. Do not leave it as it is, in case someone will "fix" it.
  • you are not filtering $_SESSION['errors'] when outputting it to the user. Right now, this is not a problem (it only contains a hard-coded string). But if you at some point change your code that this does depend on user input, this will cause problems (for example: You tried to access /cool-site/<script>alert('xss');</script>. We are sorry, but you do not have the right to access it. I would just filter it now to be sure.
  • enable HttpOnly cookies (for your cookie as well as the session cookie). I would do this in the PHP code as well as in the server settings, because you newer know if others remember to set the server settings. This mitigates the risks of XSS (although there are still risks).
  • Secure login and Do this without HTTPS: these do not go together. If you want to prevent man in the middle attacks, you will need HTTPS.

(I didn't find anything else, but that doesn't mean that there is nothing else to find.)

Apart from the security aspects, your code seems fine. Sometimes, the indentation is a bit of, and your HTML doesn't validate 100%, but I didn't see any major problems.

\$\endgroup\$
  • \$\begingroup\$ Thanks for your answer, I don't usually use php self i must have grabbed some old form code, oops! Yep, I should definitely filter the errors, just in case. I've used HTTPS in the past and understand it to be the best solution but I've also used a lot of other open source systems that advise but don't require it and never had a problem. I'll look into my validation I usually code XHTML strict. \$\endgroup\$ – CodeX Aug 26 '14 at 22:31
  • \$\begingroup\$ Proprietary attributes.. :/ \$\endgroup\$ – CodeX Aug 26 '14 at 22:33
  • \$\begingroup\$ <?php echo htmlspecialchars($_SERVER["PHP_SELF"], ENT_QUOTES, "utf-8"); ?> this or use PHP FILTER? \$\endgroup\$ – CodeX Aug 26 '14 at 22:38
  • 1
    \$\begingroup\$ htmlspecialchars is fine. See also these owasp xss prevention rules. \$\endgroup\$ – tim Aug 26 '14 at 22:58

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.