Can you vet the security of this code? log in form

PHP
1

I’m looking for security holes. The idea is that the client logs in from index.php to edit.php. For now the log-in credentials are hardcoded in the file. I need an extra set of eyes to find any security holes, or really bad design flaws if there are any.

This is index.php, the login form


<?php
session_start();
define('USERNAME','client');
define('PASS','password');
if ((isset($_POST['username'])) AND (isset($_POST['userpass'])))
{
   if ((strcmp(USERNAME, $_POST['username']) === 0) AND (strcmp(PASS, $_POST['userpass']) === 0))
   {
      $_SESSION['user'] = 'client';
      $_SESSION['pass'] = 'password';
      header('Location: edit.php');
   } else {
      unset($_POST);
      header('Location: index.php');
   }
} else {echo <<<HTML

            <form name="headerloginform" method="POST" action="index.php">
               <div class="form">
                  <label class="login-label">Username: </label><input type="text" size="8" class="login-text" name="username">
               </div>
               <div class="form">
                  <label class="login-label">Password: </label><input type="password" size="8" class="login-text" name="userpass">
               </div>
               <input type="submit"  value="Login" name="submit" class="btn">
            </form>
HTML;
}

this is edit.php the file that lets you edit the database

<?php
session_start();
define(USERNAME,'client');
define(PASS,'password');
if ((strcmp(USERNAME, $_SESSION['user']) === 0) AND (strcmp(PASS, $_SESSION['pass']) === 0)){

//a bunch of code that lets you edit      

 } else {header('Location: index.php'); exit('You lack the proper credentials to log in');}
?>
2

Dont store password in session; it’s unnecessary.

the exit message will never show up; you’re bouncing the user to a new page load immediately.

You dont need to strcmp in edit.php; check for the existance of the $_SESSION[‘user’] variable instead.

3

While not security related, your labels are technically useless. It should be


<label class="login-label" for="username">Username: </label><input type="text" size="8" class="login-text" name="username">
4

Yes, you just need to check the session variable.

And IMO, the define password should be encoded (like md5 or sha). :slight_smile:

5

Thanks for vetting the code. I took your suggestions and acted upon them.