hello people.. i ve been looking around this community, and i can see so many different ways of exploiting web applications..
but what about making a secure one..? my self i am quite a beginner in php and i ve been looking around the web for a nice user authentication tutorial, i found many, but no one is taking into consideration xss or sql injections..
so i dont know if its too much too ask, but is anyone able to provide me some good user authentication code, to get me started??

thanks in advance, and sorry if this isnt the right place for this thread!

http://sourceforge.net/projects/useratuh/ login class with encryption between frontend and backend... sql injection free also ;)

---
blog [-] microblog

you the man! ;)
gonna check it right now!thanks

kinda advanced i see.. :s

I just typed 'php security' in Google and got many useful results.
Check those out, it would be a good place to start.

Just quickly skimmed through that userauth class, and see a couple of problems already. There is a ini file called: /configs/userAtuh.ini (sic). That ini file is visible from the web, and you access it with your browser, which isn't very smart is it?

;==============
;DB Structure
;==============

;name of the users table
tableName = "users"

;name of the user-name field name
nameField = "name"

;name of the password field name
passField = "password"

;=====================
;Security Definitions
;=====================

;whether to run escaping function on input before quering (0/1)
escapeInput = 1

;what function to escape with
escapeFunction = "mysql_real_escape_string"

This is a bit dangerous, since hashing a hash increases the risk of collisions. Simply sha1() would suffice.

$encoded = sha1(sha1($pass.$name.$key))

Couple of problems here, firstly if you have a dynamic IP someone else might be able to login, 2ndly it's prone to CSRF of course.

//get the last key that was generated for this session
$ip = getenv('REMOTE_ADDR');
$key  = $this->_dba->getKeyFromDB($ip)

@rvdh: it secures some parts, by making vulnerable others (no need of table guessing)...

About the the risk of collision due to the double hashing I'm a little bit skeptic... never been a cryptographic freak :)

About the dynamic IP issue I'm also skeptic because the authentication isn't solely based upon IP address. It doesn't suffice to have the IP address, the hashed (pass,name,key) should also be supplied. It may cause problems (as you said) to those that have dynamic IP address.

About the CSRF I think you have in mind some kind of bruteforce attack (or am I wrong? *grin*)... A (re)CAPTCHA could solve the problem...

After my opinion if he wants to learn how to properly apply security, than it has to learn each issue as an isolated case. We wouldn't want to shove on his throat all at once, it would be a little hard to digest. IMHO

---
blog [-] microblog

This guy should know:-

http://www.openwall.com/phpass/

------------------------------------------------------------------------------------------------------------
"People who say it cannot be done should not interrupt those who are doing it.";
labs : [www.businessinfo.co.uk]
blog : [www.thespanner.co.uk]
Hackvertor : [hackvertor.co.uk]

@backbone

Oh yeah it are mere advisories, not zerodays ofcourse. The hashing is open for debate, but if he already salts it, there is no need for a double hassum because it really doesn't make it anymore secure.

If you don't trust sha1, I think this is an alternative sollution:

$encoded = sha1($pass.$name.$key).sha1($another_salt);

Well, if the login data is remembered in the form by a browser for example, and it has a XSS hole, then the IP can be CSRF'ed. But it really depends upon implementation and other flaws of course, but it's not far from exotic. Most vulnerabilities are stacked for compromise.

In any case: there isn't defacto security, I guess that's the point I wanted to get across in the case for someone who looks for security out of the box.

@backbone

Quote

(no need of table guessing)...

Well, if that was the case why set constants in the ini file? what if escapeInput was set to 0 when I check it out?

;whether to run escaping function on input before quering (0/1)
escapeInput = 1

That's what I would do if I was pentesting your system.

@rvdh: You got me wrong. By that I meant that the attacker by being able to see the ini file would no longer have to guess the table in case of an SQL Injection (talking about mysql 4).

And yes, it seems to me as you are assuming I made the script /:). I recommended it because it's sql injection free and loved the idea of passing encrypted data to the backend without using SSL.

---
blog [-] microblog

does anyone knows where there is 2 sha1? I mean... is there any proof somewhere than n-hahsing is better than hashing?

otherwise, as a general comment, divine's question is pretty interesting. I actually haven't see, so far, a good place where to look for secure coding information.
You can find dozen of sites about vulnerabilities, how to exploit them etc. but how to fix them and with CODE EXAMPLE, this is another one...

nEUrOO -- http://rgaucher.info -- http://twitter.com/rgaucher

It's an obscure subject...
You should check the following articles: http://websecurity.ro/blog/2007/11/02/md5md5-vs-md5/, http://stackoverflow.com/questions/348109/is-double-hashing-a-password-less-secure-than-just-hashing-it-once.

They talk about md5, but the concept is the same.

---
blog [-] microblog

@nEUrOO

Quote

otherwise, as a general comment, divine's question is pretty interesting. I actually haven't see, so far, a good place where to look for secure coding information.
You can find dozen of sites about vulnerabilities, how to exploit them etc. but how to fix them and with CODE EXAMPLE, this is another one...

True, but the problem (or danger) with code examples is that programmers use it as out of the box security, and maybe stop thinking for themselves. Furthermore you can point to all sorts of functions, but secure coding is still a process, not a patch or band-aid you stick somewhere.

I wrote an article on the matter, maybe some will find it useful:
http://insanesecurity.info/2009/02/secure-login-script-admin-panel/

@backbone,

if I may give you a small tip: use LIMIT 1 and the end of your query. Because it's possible to have two accounts with the same login data in some cases (as well as in Stefan's truncation attack.)

Normally I first do a:

 ORDER BY id ASC LIMIT 1

So that I get the first account that was registered with that combination, an account that was added later (with padded spaces for example) will fail, even if it collides in the db in question.

Oh and: use at least sha1() please! ;-) and for best practice: salt it. Then you got a pretty (more) secure login script.

A unique index against the username will also protect against both these types of attacks too

------------------------------------------------------------------------------------------------------------
"People who say it cannot be done should not interrupt those who are doing it.";
labs : [www.businessinfo.co.uk]
blog : [www.thespanner.co.uk]
Hackvertor : [hackvertor.co.uk]

Firstly I would like to thank you (both) for the feedback.

@rvdh:
I replaced the md5 hashing with sha1 hashing, while also salting the hash.

As for the second issue, the case of multiple users with the same username (truncation attack comes to mind) I didn't threat that because it has to be treated in the registration process. So I did write the script with the assumption that no other "anomalies" have slipped in before that stage.

I did however add the ORDER BY directive, just in case a coder isn't aware of truncation attacks and would use the code supplied...

---
blog [-] microblog

backbone Wrote:

Quote

@rvdh: You got me wrong. By that I meant that the attacker by being able to see the ini file would no longer have to guess the table in case of an SQL Injection (talking about mysql 4).

And yes, it seems to me as you are assuming I made the script /:). I recommended it because it's sql injection free and loved the idea of passing encrypted data to the backend without using SSL.

Well, you're still going to be sending data unencrypted from the client to the app server. It just will be encrypted from app server to the database.

-Marcin
http://tssci-security.com

Why hash a salt and then concat it to another string? From a brute force perspective it is easier to brute force (assuming we had access to the database)
$encoded = sha1($pass.$name.$key).2fd4e1c6 7a2d28fc ed849ee1 bb76e739 1b93eb12

then it is this
$encoded = sha1($pass.$name.$key.!23g^gdd z!g0(-gc fd8#9&ez1 5$rtb76$9 1b!-Zeb12)

The second example is much better as the actual string to brute force contains a larger number of possible combinations (not just a-f AND 0-9).

Also, never ever use the limit by statement on a login script

ORDER BY id ASC LIMIT 1

This is pretty much saying, we know we could get pwnd but lets just give them the first account anyway.... and if you can't guarantee a unique row identifier for each user then you need to look at the DB structure

Instead, do or get a count of the rows returned from the query and if it is more then one then its a fair bet you've been injected and bounce the user.

----------
'Just because you got the bacon, lettuce, and tomato don't mean I'm gonna give you my toast.'



Edited 1 time(s). Last edit at 02/03/2009 07:17PM by digi7al64.

Quote

Also, never ever use the limit by statement on a login script

ORDER BY id ASC LIMIT 1

This is pretty much saying, we know we could get pwnd but lets just give them the first account anyway.... and if you can't guarantee a unique row identifier for each user then you need to look at the DB structure

Instead, do or get a count of the rows returned from the query and if it is more then one then its a fair bet you've been injected and bounce the user.

I totally lost you here.

You obviously missed the issue at hand, the limit is used to only get 1 account back on a SELECT on username and password. Why? Simple. It is possible that 2 accounts are registered, this can happen due to a truncation attack, or if some part of the registration page fails to check an existing account, in this case it will be possible to login with the same credentials for 1 and the same account because it can get any of the accounts that matched, if MySQL defaults to a DESC order. (which it does) That was the whole point, and believe me, I've seen it happen. Lot of registrations can fail, and sometimes there are a bunch of accounts with blank passwords, if you do not LIMIT the records you query, you will be in trouble. What I do, does exactly what you seem to postulate: it brings back only one row IF IT EXISTS. instead of bringing more than 1 row back IF IT EXISTS. So there should be only 1 record, and that has nothing to do with the database structure since obviously the ID would be primary key, which you can't know BEFORE you have queried. But hey, what do I know, programming is my job for about 10 years now. Oh well.



Edited 2 time(s). Last edit at 02/07/2009 05:50PM by rvdh.

I understood the issue perfectly.

My problem is that rather then the system confirming only 1 record is returned from the query (by checking the number of rows returned) you force it to return only 1 row with the limit statement. Hence, if i was to undertake an attack that bypassed the input sanitizing routine you wouldn't have a clue if 100 records or 1 record was returned. Thus you can't detect a injection problem with the system login ( = fail).

Then you go on to say that this can happen due to a truncation attack or no password being present... which results in multiple rows being returned, so it is better to limit it to a single row to avoid problems.

Let me ask Ronald, with your 10 years experience do you really think it is better to just return the first row found that matches our query (since we are relying on the DESC ordering clause) rather then verifying only a single row was returned from the query (i.e. data integrity)?

.....

Let me explain it better for the benefit of the thread

Example 1 (My version) with injection (no encryption).

<?php

$link = mysql_connect("localhost", "mysql_user", "mysql_password");
mysql_select_db("database", $link);

$result = mysql_query("SELECT * FROM USERS WHERE USERID='test' AND PASS='' or 1=1", $link);
$num_rows = mysql_num_rows($result);

if ($num_rows > 1){
	//mulitple rows returned - log query for analysis - more then likely a hack attack or bad data
}elseif ($num_rows == 1){
	//do login
}else {
	//username - password incorrect
}

?>

Example 2 (Your version) with injection (no encryption).

<?php

$link = mysql_connect("localhost", "mysql_user", "mysql_password");
mysql_select_db("database", $link);

$result = mysql_query("SELECT * FROM USERS WHERE USERID='test' AND PASS='' or 1=1 LIMIT 1", $link);

if (!$result) {
    //username -password incorrect
} else {
	//do login
}

?>

As you can see the first example detects the injection, the second example using your method doesn't.

Of course, you could argue that the attacker only needs to add the limit clause to avoid detection... and whilst this is true, if someone with 10 years programming experience and your knowledge isn't aware of this basic injection detection technique then its a fair bet (most) hackers aren't either

----------
'Just because you got the bacon, lettuce, and tomato don't mean I'm gonna give you my toast.'

@digi7al64:
I think that the main point is to PREVENT injection attacks, not to process data around it :)

---
blog [-] microblog

@backbone
The game in continuing evolving and its better to be proactive in detection rather then reactive to an attack.

Also, its VERY important to note that we aren't processing data around the main function. The code I provided performed an extra check on the recordset to ensure the rowcount was equal to 1. That it is.

----------
'Just because you got the bacon, lettuce, and tomato don't mean I'm gonna give you my toast.'

It's a moot point, you suffer from injection...
$result = mysql_query("SELECT * FROM USERS WHERE USERID='admin' AND PASS='' or 1=1", $link);
Will probably only return one row as it's likely that there only is a single user with the userid admin.

In both cases the limit is subjective to the injection. An injection could be crafted to include a limit 1 constraint and thus always passing your count check. Or -- could be injected to turn the LIMIT 1 constraint into a comment. In the case of a whitebox scenario I'd judge it likely that both apps would be pwned. Better to prevent injection in the first place :)

[www.justanotherhacker.com]

@wireghoul
I know its better to prevent injection in the first place rather then try to stop it after. What I am suggesting is adding an extra check in the code to ensure only 1 record was returned from the query. Its basically a 2 liner which helps to assist in identifying if an injection occurred (which as you and I previously pointed out can be defeated).

----------
'Just because you got the bacon, lettuce, and tomato don't mean I'm gonna give you my toast.'

digi7al64 Wrote:

Quote

-------------------------------------------------------
the second example using your method doesn't.

EDIT: confused about your post, let me rephrase it:

The point was made only on a script that was properly secured already, not on a script that was flawed from the start. So there are two ways to go about this, if you already write secure code you could suffice with both, if you don't an extra check is needed to notice an injection or collision, but that was way beyond the scope of the current discussion.



Edited 4 time(s). Last edit at 02/09/2009 09:35PM by rvdh.

rvdh Wrote:
-------------------------------------------------------
> The point was made only on a script that was
> properly secured already, not on a script that was
> flawed from the start. So there are two ways to go
> about this, if you already write secure code you
> could suffice with both, if you don't an extra
> check is needed to notice an injection or
> collision, but that was way beyond the scope of
> the current discussion.

I think to assume a login script is properly secured is a big mistake and this was why I said about the row count check. Personally when I develop web apps I always try and use the Swiss Cheese Model (http://en.wikipedia.org/wiki/Swiss_Cheese_model) as a basis for development for defending against unknown errors. The row count check was simply another slice in the arsenal and to be quite honest I pretty astounded that nobody really cares about implementing after the fact detection routines into their code.

----------
'Just because you got the bacon, lettuce, and tomato don't mean I'm gonna give you my toast.'

@digi7al64

I understand what you mean now, it has similarities to the pigeonhole principle. But I when I said it I understood beforehand that all proper steps were taken to secure it, which it was in the examples of the above poster (those were not my examples). So hence the confusion. Of course I know such simple SQLi, hehe that would be so lame if I didn't, wouldn't it be? I concur with you that on preventing collisions (or preserve data integrity) counting rows afterwards is a better method, clearly. I started out with this principle when writing my first PHP scripts, but in the long run I dropped it for the main reason of laziness and experience in PHP security that I didn't felt it was necessary anymore because I was certain other scripts I've written will prevent that the pigeonhole principle will always be finite, mitigating the expected collisions. But it seems what we bth argue about is preferences, and a slight misunderstanding from both sides?