PHP login system. Safe?

Question

I am building a small cms system. The user can login and edit, delete or create a new item in the database.

My question is. Is this login system safe enough?

<?php 
session_start(); ?>
<!DOCTYPE HTML>
<html>
<head>
<meta charset="utf-8">
<title>.....</title>
</head>

<body>
<?php

    include 'koder.inc.php';


if(!isset($_POST['forsoeg'])){
    $forsoeg = 0;
    $check_user='0';
    $check_pass='0';
} else {
    $forsoeg = $_POST['forsoeg'];
    $check_user = $_POST['username'];
    $check_pass = $_POST['password']; }

    if($check_user != $username || $check_pass != $password)     {
        if($forsoeg >3){
            exit("<p>Wrong password or username <br /><br />
            <a href='admin_logon.php'>back to login</a></p>");}


            $forsoeg ++;  

            ?>
<h1>Login</h1>
<form action="<?php echo $_SERVER['PHP_SELF']; ?>" method="post" name="form">
  <p>
    <label for="username">username:</label>
    <br />
    <input title="username:" type="text" name="username"  />
  </p>
  <p>
    <label for="password">Password:</label>
    <br />
    <input title="Skriv dit password" type="text" name="password"  />
  </p>
  <p>
    <input class="knap" type="submit" name="Send" value="Login" />
  </p>
  <input type="hidden" name="forsoeg" value="<?php echo $forsoeg; ?>"  />
</form>
<?php

    } else {
        $_SESSION['logon']="ok";

        echo "Login ok..<br />
        <a href='administration.php'>Go to admin page</a>"; } ?>
</body>
</html>

I then include the fil koder.inc.php

<?php 

 $username = "test";
 $password = "123456";


?>

And on the pages that need a valid user i start the page with

<?php 
session_start(); ?>

user name password stored in file in plain text - you only have one user? — , Aug 09 '12 at 10:23
@Dagon You can't use SQL injection with plain text files :P teehee — Fluffeh, Aug 09 '12 at 10:25
@user name password stored in file in plain text.. Is this a question? For now yes. But in time i will have to create more users. — Kasper, Aug 09 '12 at 10:26
use a db (text files don't scale), don't store plain text passwords -this topic has been covered a few million times — , Aug 09 '12 at 10:28
Putting the number of attempts in the hidden field? Ok, I'll just reset that to `0` each time. — Leigh, Aug 09 '12 at 10:34

score 1 · Answer 1 · answered Aug 09 '12 at 10:33

1

If the user credentials are stored in plain text files, it is very unsafe. And this method is very uncommon for the today systems. I recommend you to use a database for the users and store their password in a hashed format, like md5. This is also better when you have more than just one user!

answered Aug 09 '12 at 10:33

tuxtimo

2,730
20
29

why should it not be recommended? – tuxtimo Aug 09 '12 at 10:44
3

Actually, md5()/sha1()/sha256()/sha512() are ALL inappropriate hashes for storing a password - they require trivial time to test. 'bcrypt' or 'pbkdf2' are much better as they will scale with advances in technology. – CubicleSoft Aug 09 '12 at 13:58

score 1 · Answer 2 · answered Aug 10 '12 at 09:22

In addition to the already-stated points, you have HTML injection vulnerabilities (leading to cross-site scripting attacks) here:

<form action="<?php echo $_SERVER['PHP_SELF']; ?>"

and here:

<input type="hidden" name="forsoeg" value="<?php echo $forsoeg; ?>"  />

You must use htmlspecialchars() every time you output text content into HTML markup. For example:

function h($s) {
    echo htmlspecialchars($s, ENT_QUOTES, 'utf-8');
}

...

<form action="<?php h($_SERVER['PHP_SELF']); ?>" ...
<input type="hidden" name="forsoeg" value="<?php h($forsoeg); ?>"  />

Kao · Accepted Answer · 2012-09-04T08:19:01.363

-1

If you're going for a multi user setup, you should migrate from a plain text file, and use a database instead.

For storing passwords in the database you would need to first encrypt it, using PBKDF2 etc. then add a salt, and encrypt it again. The salt has to be random, it is safe to keep this in the database stored in plaintext along with the password.

To verify a log in, you would pull the password from the database, and match it with an encrypted version of the password stored in the $_POST variable.

edited Sep 04 '12 at 08:19

answered Aug 09 '12 at 10:36

Kao

2,242
3
22
31

Okey, ill will have to find som tutorials. This is way to advance for me. Thx for the answer. – Kasper Aug 09 '12 at 10:40
1

Also, you should store the attempts of logins, in a session variable. the hidden post field can just be manipulated. – Kao Aug 09 '12 at 10:42
1

See my reply to tuxtimo, but this is misleading and potentially just as dangerous as storing passwords as plain text. Salting does little given today's technology and the recommendation to use sha512 in combination will lead to a false sense of security. – CubicleSoft Aug 09 '12 at 14:01
The claim is valid, SHA is not good enough for password storage, use BCrypt, SCrypt or PKBDF2. a quick source: http://security.stackexchange.com/q/211/2113 – Jacco Sep 03 '12 at 13:52
Updated my answer, to provide a PBKDF2 link instead of SHA. – Kao Sep 04 '12 at 08:19

PHP login system. Safe?

3 Answers3