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!

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?

@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

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.

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...

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.

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.

@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...

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.

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.

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)?

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

@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.

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 :)

@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).

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.

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.

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?