Is this secure? (Help w/ login page)

Discussions of secure PHP coding. Security in software is important, so don't be afraid to ask. And when answering: be anal. Nitpick. No security vulnerability is too small.

Moderator: General Moderators

Post Reply
completely-stumped
Forum Newbie
Posts: 5
Joined: Tue Nov 08, 2011 10:32 am

Is this secure? (Help w/ login page)

Post by completely-stumped »

I am an absolute beginner with PHP, so please bear with me. I am looking for a simple login page where I can add 3-4 users.

After searching all over and not really finding a clear answer I stumbled across this video "http://www.youtube.com/watch?v=d4qdqUpenWk" and following those instructions setup the following files on my web space. It "appears" to work, but I have no clue if this is secure or not.

One concern I have is that as you can see in "checklogin.php" the SQL pw is in clear text (no md5 hash?). I've put that file in a separate folder and locked it off with .htaccess, but I'm not clear on if that is enough. A second concern is "injection attacks"? Please help me.

-------------
mainlogin.php

Code: Select all

<html>
  <head>

    <title>Login</title>

    <style type="text/css">
    #loginform {
      border: 2px solid #ACA7A4 ;
      background-color: #878381 ;
      width: 280px ;
      }

    #loginform form {
      margin: 5px ;
      }

    label {
      display: block ;
      width: 90px ;
      float: left ;
      clear: both ;
      }

    label, input {
      margin-bottom: 4px ;
      }

    </style>
  </head>
<body bgcolor="#000000">

<div id="loginform">
  <form method="post" action="../php/checklogin.php" name="form1">

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

      <label for="password">Password:</label>
      <input type="password" name="mypassword" id="password" /> <!-- Change "type" to "password" to create *** field -->
      <input type="submit" name="submit" value="Login" />

  </form>
</div>

</body>
</html>
-------------
checklogin.php

Code: Select all

<?
  $host = "host.com" ;
  $username = "username" ;
  $password = "password" ;
  $db_name = "database" ;
  $tbl_name = "table" ;

  mysql_connect ($host, $username, $password) or die(mysql_error("Can't connect"));
  mysql_select_db ($db_name) or die (mysql_error());

$myusername = $_POST['myusername'];
$mypassword = $_POST['mypassword'];

$sql = "SELECT * FROM $tbl_name WHERE username='$myusername' and password='$mypassword'";
$result = mysql_query($sql);

$count = mysql_num_rows($result);

if($count==1) {
    session_register("myusername");
    session_register("mypassword");
    header("location:/login_success.php");
    }
    else {
      echo "Wrong Username or Password";
      }
?>
-------------
login_success.php

Code: Select all

<?
  session_start();
    if(!session_is_registered(myusername)) {
      header("location:mainlogin.php");
      }
?>

<html>
  <head> <title>Welcome</title>
  </head>

  <body>
  <h1>Login Successful</h1>
    <p>
    <a href="logout.php">Log Out!</a></p>
  </body>
</html>
-------------
logout.php

Code: Select all

<?
  session_start();
  session_destroy();
?>

<html>
  <head> <title>Goodbye</title>
  </head>
  
  <body>
  <h1>You've been logged out.</h1>
  </body>
</html>
User avatar
Celauran
Moderator
Posts: 6427
Joined: Tue Nov 09, 2010 2:39 pm
Location: Montreal, Canada

Re: Is this secure? (Help w/ login page)

Post by Celauran »

You're right that storing the password in plain text in the database is a bad idea. Storing an md5 hash is definitely better, but still not particularly secure. I'd recommend adding salt and pepper and using a stronger hashing algorithm. Use a common salt for all your users (just a string you store somewhere) and a pepper that's unique to each user (their user ID, account creation time, etc), concatenate the whole mess and hash the result.
User avatar
flying_circus
Forum Regular
Posts: 732
Joined: Wed Mar 05, 2008 10:23 pm
Location: Sunriver, OR

Re: Is this secure? (Help w/ login page)

Post by flying_circus »

It looks like you trust your users to provide correct information.

How does your code handle incorrect credentials?
How does your code handle incomplete credentials?
How does your code handle specials characters?

Code: Select all

/* As a user, in your form, I type the following info:
 username = *
 password = ' OR 1='1

  # My point is, you need to learn about SQL Injection
*/


$myusername = $_POST['myusername'];
$mypassword = $_POST['mypassword'];

$sql = "SELECT * FROM $tbl_name WHERE username='$myusername' and password='$mypassword'";
User avatar
Mordred
DevNet Resident
Posts: 1579
Joined: Sun Sep 03, 2006 5:19 am
Location: Sofia, Bulgaria

Re: Is this secure? (Help w/ login page)

Post by Mordred »

session_register() was an old technique even back in 2002 and it amuses me to no end to still see it in "tutorials", especialy "security" "tutorials" (even the quotes should have quotes :) )

Apart from the sql injection and cleartext passwords already pointed out, you should care for session fixation (use session_regenerate_id after login) and the login checking code -> call exit() after header('Location...'), otherwise any code below will still be executed.
User avatar
social_experiment
DevNet Master
Posts: 2793
Joined: Sun Feb 15, 2009 11:08 am
Location: .za

Re: Is this secure? (Help w/ login page)

Post by social_experiment »

When using mysql_connect() an option is to use the error control operator (@) in front of the function to stop any sensitive information from being displayed in case the database connection cannot be made. Same goes for the mysql_select_db() statement and the die(mysql_error()) at the end of that statement; it's good to have it in production but if your site is live, remove it or deal with the resulting error in another, "clean" way.

Another point, maybe not directly related to security but more an efficiency point is to use sql's COUNT() function to find a matching row within the database. It involves different code in retrieving the amount of rows returned but you don't have to use mysql_num_rows().

Code: Select all

<?php
 $sql = "SELECT COUNT(id) FROM usertbl WHERE password = '$password' AND username = '$username' ";
?>
Don't use short tags ( <? ?> ).
“Don’t worry if it doesn’t work right. If everything did, you’d be out of a job.” - Mosher’s Law of Software Engineering
completely-stumped
Forum Newbie
Posts: 5
Joined: Tue Nov 08, 2011 10:32 am

Re: Is this secure? (Help w/ login page)

Post by completely-stumped »

Celauran wrote:
Storing an md5 hash is definitely better, but still not particularly secure. I'd recommend adding salt and pepper and using a stronger hashing algorithm. Use a common salt for all your users (just a string you store somewhere) and a pepper that's unique to each user (their user ID, account creation time, etc), concatenate the whole mess and hash the result.
Way over my head. :(
flying_circus wrote:IHow does your code handle incorrect credentials?
How does your code handle incomplete credentials?
How does your code handle specials characters?
I have no idea. Like I said I'm completely new to PHP and just followed an instruction page :(
Mordred wrote:session_register() was an old technique even back in 2002 and it amuses me to no end to still see it in "tutorials", especialy "security" "tutorials"
Not so amusing for us newbs. :(



This seems like such a simple thing, isn't there some "drop in" solution somewhere? It seems the answer I found is garbage, but where do I look for a real answer?
User avatar
flying_circus
Forum Regular
Posts: 732
Joined: Wed Mar 05, 2008 10:23 pm
Location: Sunriver, OR

Re: Is this secure? (Help w/ login page)

Post by flying_circus »

completely-stumped wrote:This seems like such a simple thing, isn't there some "drop in" solution somewhere? It seems the answer I found is garbage, but where do I look for a real answer?
This is, in my opinion, the single biggest misconception that people have. Oh, it's just a login system, they're everywhere, it should be a simple task.

I responded to a thread not too long ago, and I was pretty intimidated to provide an answer on such a difficult topic. I've been waiting for a mentor to tear it apart, but it hasnt happened yet.

Take a look at this link and scroll down until you find my post. Have a quick read and make your own decisions.

http://www.devnetwork.net/viewtopic.php?f=1&t=129885
User avatar
social_experiment
DevNet Master
Posts: 2793
Joined: Sun Feb 15, 2009 11:08 am
Location: .za

Re: Is this secure? (Help w/ login page)

Post by social_experiment »

flying_circus wrote:completely-stumped wrote:This seems like such a simple thing, isn't there some "drop in" solution somewhere? It seems the answer I found is garbage, but where do I look for a real answer?This is, in my opinion, the single biggest misconception that people have. Oh, it's just a login system, they're everywhere, it should be a simple task.
I agree with flying_circus; it may seem like a simple task but you have to rememeber that not only your 'non-malicious' users will be visiting the page. Once the page is uploaded it's free game; you have very little control over who sees it unless you create a domain only you and your users know and keep it completely secret between the people using it. That could be seen as security through obscurity but it's not a great way to protect your system.
completely-stumped wrote:Way over my head.
Search the forum; security is a very popular topic and this type of question has been asked before; also look at the urls in signatures of other forum members; very interesting reads in there.
completely-stumped wrote:I have no idea. Like I said I'm completely new to PHP and just followed an instruction page
Look at functions like trim(), which removes whitespace (or other characters) from a string or something like preg_match(). Those are simple questions to ask yourself:
1. What do i constitute as 'valid' input i.e numbers, strings, non-alphanumeric characters?
2. If a users enters no data, enters no username, enters no password, what does my script do?
3. Certain characters can be used for attacks like cross-site scripting, how will i handle things like >, <, ', " ?
completely-stumped wrote:Not so amusing for us newbs.
I would rather get a bit of egg on my face here, asking and learning than having it happen on a live site. That could potentially ruin your development career. Something else i also marvel at sometimes when browsing the forum is the level of difficulty that newbies attempt on their first script. I respect the fact that you wish to learn php but you shouldn't use code without understanding it completely. If you don't understand code, don't accept it on face-value; have a look on forums like devnet, do some more research. Security (and login security) is such a hot topic i'd be surprised if you didn't find a better example than the one you currently have.
“Don’t worry if it doesn’t work right. If everything did, you’d be out of a job.” - Mosher’s Law of Software Engineering
completely-stumped
Forum Newbie
Posts: 5
Joined: Tue Nov 08, 2011 10:32 am

Re: Is this secure? (Help w/ login page)

Post by completely-stumped »

Thanks everyone for your informed replies, you've all been most helpful.


flying_circus wrote:
completely-stumped wrote:This seems like such a simple thing, isn't there some "drop in" solution somewhere? It seems the answer I found is garbage, but where do I look for a real answer?
This is, in my opinion, the single biggest misconception that people have. Oh, it's just a login system, they're everywhere, it should be a simple task.
My thought (obviously wrong :) ) was that since PHP is a set language and SQL is a fairly standard db language, there would be lots of ready-to-go and simple drop ins for this task. After all I found .htaccess simple so hoped this would be equally straight forward. I now understand that there's more too it than that.
That's the danger of being a "newb" to any field, we just don't know what we don't know :)

social_experiment wrote:... Once the page is uploaded it's free game; you have very little control over who sees it unless you create a domain only you and your users know and keep it completely secret between the people using it. That could be seen as security through obscurity but it's not a great way to protect your system....
I come from a networking background, so I know exactly what you mean :)
3. Certain characters can be used for attacks like cross-site scripting, how will i handle things like >, <, ', " ?
*nods* yeah I had read some about special characters being used to do what I seem to recall was an "injection " attack. Since what I posted had no way of dumping or dealing with those I was a bit skeptical.




Well one last time, thanks all.
Post Reply