Noobish password protection?

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
trobale
Forum Newbie
Posts: 24
Joined: Mon Apr 23, 2007 11:42 am

Noobish password protection?

Post by trobale »

I was wondering how secure this kind of password protection is:

First when the user logs in (password and username checked) a session is started and his/her user id is registered to the session

Code: Select all

$_SESSION['User_ID']
Also his/her ip adress is registered to the session

Code: Select all

$_SESSION['IP']
Then I have the following code on every page that is password protected:

Code: Select all

<?php
session_start();

if(!isset($_SESSION['User_ID']) || $_SERVER['REMOTE_ADDR']!=$_SERVER['IP']){
          
        // DISPLAY LOGIN SCREEN OR SOMETHING SIMILAR
}
else {

       // DISPLAY PAGE CONTENT
}
?>
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

There's no password protection at all in the code you've posted. The "protection" appears to only happen once, at the login point.

Be aware that it is perfectly valid for a users' IP address to change during their session. In fact, that is the norm for AOL users.
trobale
Forum Newbie
Posts: 24
Joined: Mon Apr 23, 2007 11:42 am

Post by trobale »

The user can only get the $_SESSION['User_ID'] variable stored if has logged in. I check that on every page. How can he get that variable stored if he doesn't login? :?

If the ip address changes I'll probably have to take it away. I had it there just because I read that SESSION id:s could possibly be accessed...

I have tested this type of protection on my site and it appears to work quite nicely. If you try to access a page without logging in it will display the login form.

I asked this question because I don't know how secure this kind of protection. As I said it works nicely on my page, but I don't know how hacker safe it is...
Zu
Forum Commoner
Posts: 33
Joined: Wed Dec 06, 2006 4:21 am

Post by Zu »

trobale wrote:The user can only get the $_SESSION['User_ID'] variable stored if has logged in. I check that on every page. How can he get that variable stored if he doesn't login? :?
There is no password protection in the code snippets you posted, only variable testing. Perhaps show the code that deals with inputted password, such as input filtering and database querying.
trobale wrote:If the ip address changes I'll probably have to take it away. I had it there just because I read that SESSION id:s could possibly be accessed...
As feyd said, AOL users in particular access a server farm whereby they might be assigned a different machine.
trobale wrote:I have tested this type of protection on my site and it appears to work quite nicely. If you try to access a page without logging in it will display the login form.
Yes, it will work. How secure is it? Well, not very ;)

An evil-user would only need to grab a valid session ID and he could appear to be logged in as someone else.
trobale
Forum Newbie
Posts: 24
Joined: Mon Apr 23, 2007 11:42 am

Post by trobale »

The Code I check password and username with:

Code: Select all

if(isset($_POST['username']) && isset($_POST['password'])){ // CHECKS IF USERNAME AND PASSWORD ENTERED

require('common/connect.php'); // CONNECT TO DATABASE

$username=$_POST['username'];

$password=$_POST['password'];

$query=mysql_query("SELECT `User_ID`,`Username`,`Password` FROM `users` WHERE (Username COLLATE latin1_general_cs ='$username') AND (Password COLLATE latin1_general_cs ='$password')");

mysql_close($con);

if(mysql_num_rows($query)==0){ // IF NUM_ROWS== 0 --> WRONG USERNAME OR PASSWORD

$alert="alert('Wrong password or username!')";

$redirect_to="login.php";

}

else { // USERNAME AND PASSWORD OK

session_start();

$row=mysql_fetch_array($query);

$_SESSION['User_ID']=$row['User_ID'];

$_SESSION['IP']=$_SERVER['REMOTE_ADDR']; 

$alert="alert('Logged in succesfully!')";

$redirect_to="my_account.php";
}

// REST OF SCRIPT
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

Your code has SQL injection opportunities for easy exploitation.
trobale
Forum Newbie
Posts: 24
Joined: Mon Apr 23, 2007 11:42 am

Post by trobale »

I use mysql_real_escape_string() for that (shouldn't it work?).

I was just wondering about the security of my password protection and it seems that it is null 8)

I thought that testing session variables with isset() would work since I don't know any way to register that session variable without logging in.

Is my login script password protection then secure?

And as I said, It worked nice for me...
Last edited by trobale on Mon Apr 30, 2007 4:02 am, edited 1 time in total.
User avatar
shiznatix
DevNet Master
Posts: 2745
Joined: Tue Dec 28, 2004 5:57 pm
Location: Tallinn, Estonia
Contact:

Post by shiznatix »

I don't see you using mysql_real_escape_string anywhere in that code. You just have the user input put into a local variable and then stright into the query.
User avatar
shiflett
Forum Contributor
Posts: 124
Joined: Sun Feb 06, 2005 11:22 am

Post by shiflett »

There are SQL injection vulnerabilities in your code, which feyd has already mentioned. Therefore, it makes little sense to offer up a demo sans query for people to prove to you that it is vulnerable. It's also worth considering that other people's time is valuable, and asking them to prove themselves is not being very respectful. Answers here are offered for free. It makes sense to verify them, but that's your job.

Regarding your inspection of $_SESSION['User_ID'], this technique is worth very little, because you're setting this value whenever someone logs in. If an attacker is going to try to hijack a session, it's probably not going to be an anonymous user's session.

You require $_SERVER['IP'] to be the origin of the current request, and although I assume you mean $_SESSION['IP'], requiring this to be static is going to cause many legitimate users trouble.

Hope that helps.
trobale
Forum Newbie
Posts: 24
Joined: Mon Apr 23, 2007 11:42 am

Post by trobale »

Thanks for the answers. I forgot to write the mysql_real_escape string() function to my sample code but I am using that (and I hope that it is enough)!

I'm quite a beginner at php coding and therefore I don't understand all your arguments against my 'password protection'.

I had that ip check there just because I read that hackers can hijack sessions. If the ip address isn't the same as the ip address with which the session was registered the user can't access the site (and therefore hijacking is impossible?).

Code: Select all

if(!isset($_SESSION['User_ID']) || $_SERVER['REMOTE_ADDR']!=$_SERVER['IP']){
         
        // DISPLAY LOGIN SCREEN OR SOMETHING SIMILAR
}
else {

       // DISPLAY PAGE CONTENT
}
Someone wrote that especially AOL users change their ip address during browsing. This doesn't make the script more vulnerable (I think :roll: ) since the user check will then fail and login screen is shown. Of course this makes that AOL users have to login several times which is too bad :( .
Zu
Forum Commoner
Posts: 33
Joined: Wed Dec 06, 2006 4:21 am

Post by Zu »

trobale wrote:Someone wrote that especially AOL users change their ip address during browsing. This doesn't make the script more vulnerable (I think :roll: ) since the user check will then fail and login screen is shown. Of course this makes that AOL users have to login several times which is too bad :( .
I am sure the millions upon millions of AOL users, and other ISP's, will love you for that.
User avatar
superdezign
DevNet Master
Posts: 4135
Joined: Sat Jan 20, 2007 11:06 pm

Post by superdezign »

You know, BEFORE we, as web developers, learned about security techniques, we typically learned about usability and accessibility. Otherwise, the internet would be a hassle.

I understand that no one likes to be told that they are wrong, but you need to rethink your security and protection methods, especially if your website isn't one that most experienced hackers would even care to access. If you REALLY want to learn about security though, become a hacker. It'll make sense then.

You can control a lot more on the client-side than you'd think.
Z3RO21
Forum Contributor
Posts: 130
Joined: Thu Aug 17, 2006 8:59 am

Post by Z3RO21 »

Code: Select all

if(!isset($_SESSION['User_ID']) || $_SERVER['REMOTE_ADDR']!=$_SERVER['IP']){
         
        // DISPLAY LOGIN SCREEN OR SOMETHING SIMILAR
}
else {

       // DISPLAY PAGE CONTENT
}
This does not do anything. $_SERVER does not have an index with the key 'IP', unless it is a depreciated value. Still this code does nothing security wise...
Last edited by Z3RO21 on Mon Apr 30, 2007 5:39 pm, edited 1 time in total.
User avatar
RobertGonzalez
Site Administrator
Posts: 14293
Joined: Tue Sep 09, 2003 6:04 pm
Location: Fremont, CA, USA

Post by RobertGonzalez »

May not seem like much to you now, but you realize that your query suggests that you are storing passwords in plain text in your database. That is insecure in and of itself. At least try to thwart someones attempt and getting your users information.
Z3RO21
Forum Contributor
Posts: 130
Joined: Thu Aug 17, 2006 8:59 am

Post by Z3RO21 »

Everah wrote:May not seem like much to you now, but you realize that your query suggests that you are storing passwords in plain text in your database. That is insecure in and of itself. At least try to thwart someones attempt and getting your users information.
Some useful functions pertaining to Everah's post are md5(), sha1(), and hash().

I hope these help you!
Post Reply