Page 1 of 2
Login Code... Will it work?
Posted: Sun Feb 04, 2007 7:16 pm
by Kureigu
feyd | Please use Code: Select all
and [syntax="..."] tags where appropriate when posting code. Your post has been edited to reflect how we'd like it posted. Please read: [url=http://forums.devnetwork.net/viewtopic.php?t=21171]Posting Code in the Forums[/url] to learn how to do it too.[/color]
I wrote this after doing some research and trying to understand tutorials, etc.
I'm not sure if it will work. Can anyone see any major mistakes or flaws?
Code: Select all
<?php
function db_login($uname,$pword){
$dbHost = "localhost";
$dbUser = "";
$dbPass = "";
$dbDatabase = "userbeta";
$dbTable = "userbeta";
$db = mysql_connect("$dbHost","$dbUser","$dbPass") or die("Error accessing user database. Please try again.");
mysql_select_db("$dbDatabase",$db) or die ("Could not select database");
$out = mysql_query("SELECT * FROM $dbTable WHERE username=$uname AND password=$pword", $db);
$rows = mysql_num_rows($out);
if($rows > 0){
while( $row = mysql_fetch_array($out) ){
session_start();
session_register('username');
echo ("login as " . $uname . "was successful!");
}
}else{
echo ("Wrong username or password!");
}
}
// Begin main
$uname = $_POST['uname'];
$pword = md5($_POST['pword']);
if(!isset($uname) || !isset($pword)){
header("location: login.php");
}elseif(empty($uname) || empty($pword)){
header("location: login.php");
}else{
db_login($uname,$pword);
}
?>
Thanks.
Note: Changed the above to updated version.
feyd | Please use Code: Select all
and [syntax="..."] tags where appropriate when posting code. Your post has been edited to reflect how we'd like it posted. Please read: [url=http://forums.devnetwork.net/viewtopic.php?t=21171]Posting Code in the Forums[/url] to learn how to do it too.[/color]
Posted: Sun Feb 04, 2007 7:25 pm
by wildwobby
Looks good except why is the password just an md5() of the username?
Posted: Thu Feb 08, 2007 5:27 am
by Kureigu
Can someone explain why I get this error?
Warning: mysql_num_rows(): supplied argument is not a valid MySQL result resource in c:\phpdev\www\mmocc\session.php on line 15
It's wierd. I get it in my register script aswell when I use mysql_num_rows();
Anyone know what I'm doing wrong?
Posted: Thu Feb 08, 2007 5:38 am
by louie35
Caution
If you want your script to work regardless of register_globals, you need to instead use the $_SESSION array as $_SESSION entries are automatically registered. If your script uses session_register(), it will not work in environments where the PHP directive register_globals is disabled.
http://ie2.php.net/function.session_register
Posted: Thu Feb 08, 2007 9:13 am
by NEWDAY
Kureigu wrote:Can someone explain why I get this error?
Warning: mysql_num_rows(): supplied argument is not a valid MySQL result resource in c:\phpdev\www\mmocc\session.php on line 15
It's wierd. I get it in my register script aswell when I use mysql_num_rows();
Anyone know what I'm doing wrong?
Check whether the fields in your database are exactly named username and password as you mentioned in the query:
$out = mysql_query("SELECT * FROM $dbTable WHERE
username=$uname AND
password=$pword", $db);
hint: dont give the database and the table the same name so you wont get confused.
Posted: Thu Feb 08, 2007 9:46 am
by feyd
$uname and $pword allow for SQL injection.
Posted: Thu Feb 08, 2007 9:48 am
by Mordred
You have an SQL injection on $uname.
Posted: Thu Feb 08, 2007 9:50 am
by Kureigu
SQL injection?
Also, the names are not different from the DB.
Posted: Thu Feb 08, 2007 9:52 am
by feyd
We have many many threads on injection, please search for them.
Posted: Thu Feb 08, 2007 9:57 am
by jolinar
wildwobby wrote:Looks good except why is the password just an md5() of the username?
Agreed, you should use, at least, SHA1 (SHA256 if your server supports it, though this is often not the case)
As for SQL injection, a simple routing that looks for "evil" input using something like regex could work. Here's an example of the quick (and very) dirty system I use.
Code: Select all
function check_input($input) {
if (preg_match('/{|}|"|<|>|\.\//',$input)){
return true;
}
return false;
}
function scan($input) {
if( check_input($input)==true ) {
print "<h3>Error, reserved characters present database query!</h3>\n";
exit;
}
}
The username sould be run through a set of funtions like this. The idea is "treat all input as evil" (password shouldn't be a problem as it gets hashed)
Posted: Thu Feb 08, 2007 10:05 am
by feyd
jolinar wrote:Agreed, you should use, at least, SHA1 (SHA256 if your server supports it, though this is often not the case)
Most servers will support it if you use
my class.

Posted: Thu Feb 08, 2007 10:15 am
by Mordred
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.
Posted: Fri Feb 09, 2007 11:44 am
by Kureigu
I appreciate all of the advice, and I'll take time to read over it.
Morbred, I did neither because I've never heard of either of them. You should be able to tell from the script I'm not very experienced with PhP so need alittle help here and there.
Now can someone tell me what I have to do to fix this problem?
I don't really need being told what I should do to make it better, I just want it working first. I purposely made it basic to start because I had never done a login script before.
Re: Login Code... Will it work?
Posted: Fri Feb 09, 2007 12:35 pm
by Christopher
There are no quotes around your username and password -- hence the query fails so no MySQL result resource. I would suggest something like this:
Code: Select all
$uname = mysql_real_escape_string($uname);
$pword= mysql_real_escape_string($pword);
$out = mysql_query("SELECT * FROM $dbTable WHERE username='$uname' AND password='$pword'", $db);
Posted: Fri Feb 09, 2007 2:52 pm
by RobertGonzalez
Maybe something like this for your code. Of course, this needs review and should account for greater checking of input data for validation reasons...
Code: Select all
<?php
session_start();
function db_login($uname, $pword)
{
// There could stand to be more validation than this
$uname = mysql_real_escape_string($uname);
$pword = mysql_real_escape_string($pword);
// Handle empty values
if (empty($uname) || empty($pword))
{
// Use this area to handle errors
return false;
}
$dbHost = "localhost";
$dbUser = "";
$dbPass = "";
$dbDatabase = "userbeta";
$dbTable = "userbeta";
if (!$db = mysql_connect($dbHost, $dbUser, $dbPass))
{
die("Error accessing user database. Please try again.");
}
if (!mysql_select_db($dbDatabase, $db))
{
die("Could not select database");
}
// Backticks help prevent reserved field name clashes
// Also, since this is a single login, you should limit the return result to 1
$sql = "SELECT * FROM `$dbTable` WHERE `username` = '$uname' AND `password` = '$pword' LIMIT 1"
//execute the query
if (!$out = mysql_query($sql, $db))
{
// You can leave out the mysql_error() call for production
die('Could not execute the query: ' mysql_error());
}
if (mysql_num_rows($out) == 1)
{
$_SESSION['username'] = $uname;
return true;
}
else
{
return false;
}
}
// Begin main
$uname = isset($_POST['uname']) ? $_POST['uname'] : '';
$pword = isset($_POST['pword']) ? md5($_POST['pword']) : '';
if (empty($uname) || empty($pword))
{
header("location: http://www.fuldomainurl.com/login.php");
exit;
}
if (!$login = db_login($uname, $pword))
{
echo 'There was a problem';
}
else
{
echo 'Sweet, you are in!';
}
?>