Login Code... Will it work?

PHP programming forum. Ask questions or help people concerning PHP code. Don't understand a function? Need help implementing a class? Don't understand a class? Here is where to ask. Remember to do your homework!

Moderator: General Moderators

User avatar
jolinar
Forum Commoner
Posts: 61
Joined: Tue May 24, 2005 4:24 pm
Location: in front of computer

Post by jolinar »

Mordred wrote:jolinar, your function is fine, but it is not enough. All data should be mysql_real_escape_string()-ed before putting it in a mysql_query(). If you have to stop and think if your check was enough, this means it's not enough.

Your function is about validation, which is a part of the code logic level. SQL Injection is an attack on the syntax level, and should be stopped by syntactical means. Proper escaping is one, writing SQL statements with proper syntax is another, and Kureigu did neither.

Btw, after $pword = md5($_POST['pword']); $pword is safe enough, on the code logic level it is fine. But remember what I told you - if you have to stop and think if it's enough, then it's not enough! Good coding practice dictates that you escape it as well.
Very good point, I've just started using that now. PHP 5 has a very nifty feature for this, the foreach iterator can be used to modify superglobals (probably not good practice)

Code: Select all

foreach($_GET as &$tmp) {
	$tmp = mysql_escape_string($tmp);
}
foreach($_POST as &$tmp) {
	$tmp = mysql_escape_string($tmp);
}
That, specifically, is the result of a conversation I had with a developer I know. My site uses a MySQL table to store some of the site's settings (in a similar vein to PHP Nuke, my coding style is horrible)

The problem suggested was that if I forgot to scan/escape the input then it would be vulnerable to a SQL injection attack. This approach scans EVERYTHING in $_GET and $_POST before the rest of the program even touches those variables.

Since this is a PHP5 specific feature, I've had to include a check at the start of the program that stops it and displays a version error if it detects anything<5 (In case I accidentally load it on to a machine running PHP4)
User avatar
Mordred
DevNet Resident
Posts: 1579
Joined: Sun Sep 03, 2006 5:19 am
Location: Sofia, Bulgaria

Post by Mordred »

jolinar, wash your hands with soap ;)

Don't ever do this kind of thing, it is plain wrong. The PHP team tried to do something similar (read on magic_quotes) and it was a disaster, security got no better, but writing code with magic_quotes on became a nuisance.

Escape for mysql ONLY if you're going to put the variable in mysql_query.

As for modifying arrays, PHP4 can do it like this:

Code: Select all

foreach ($array as $key=>$value)
   $array[$key] = $value . ' was changed';
User avatar
jolinar
Forum Commoner
Posts: 61
Joined: Tue May 24, 2005 4:24 pm
Location: in front of computer

Post by jolinar »

I stand corrected :(
Kureigu
Forum Newbie
Posts: 8
Joined: Wed Jan 31, 2007 2:45 pm

Post by Kureigu »

Ok guys, I started over with the login page. There are some similarities, but little chance for SQL injection.

Oh, also. It works.
I also created a register script and a logout script.

I just have to now work out how to disallow certain username and password characters, and check that the email address is of a valid format.

Right now I'm working on a "create profile" section using text areas. I'll re-post when It's done.
Post Reply