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

Kureigu
Forum Newbie
Posts: 8
Joined: Wed Jan 31, 2007 2:45 pm

Login Code... Will it work?

Post by Kureigu »

feyd | Please use

Code: Select all

,

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

,

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]
Last edited by Kureigu on Thu Feb 08, 2007 5:37 am, edited 6 times in total.
wildwobby
Forum Commoner
Posts: 66
Joined: Sat Jul 01, 2006 8:35 pm

Post by wildwobby »

Looks good except why is the password just an md5() of the username?
Kureigu
Forum Newbie
Posts: 8
Joined: Wed Jan 31, 2007 2:45 pm

Post 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?
User avatar
louie35
Forum Contributor
Posts: 144
Joined: Fri Jan 26, 2007 8:40 am
Location: Dublin
Contact:

Post 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
NEWDAY
Forum Newbie
Posts: 12
Joined: Wed Jan 31, 2007 5:15 pm

Post 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.
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

$uname and $pword allow for SQL injection.
User avatar
Mordred
DevNet Resident
Posts: 1579
Joined: Sun Sep 03, 2006 5:19 am
Location: Sofia, Bulgaria

Post by Mordred »

You have an SQL injection on $uname.
Kureigu
Forum Newbie
Posts: 8
Joined: Wed Jan 31, 2007 2:45 pm

Post by Kureigu »

SQL injection?

Also, the names are not different from the DB.
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post by feyd »

We have many many threads on injection, please search for them.
User avatar
jolinar
Forum Commoner
Posts: 61
Joined: Tue May 24, 2005 4:24 pm
Location: in front of computer

Post 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)
User avatar
feyd
Neighborhood Spidermoddy
Posts: 31559
Joined: Mon Mar 29, 2004 3:24 pm
Location: Bothell, Washington, USA

Post 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. :)
User avatar
Mordred
DevNet Resident
Posts: 1579
Joined: Sun Sep 03, 2006 5:19 am
Location: Sofia, Bulgaria

Post 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.
Kureigu
Forum Newbie
Posts: 8
Joined: Wed Jan 31, 2007 2:45 pm

Post 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.
User avatar
Christopher
Site Administrator
Posts: 13596
Joined: Wed Aug 25, 2004 7:54 pm
Location: New York, NY, US

Re: Login Code... Will it work?

Post 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);
(#10850)
User avatar
RobertGonzalez
Site Administrator
Posts: 14293
Joined: Tue Sep 09, 2003 6:04 pm
Location: Fremont, CA, USA

Post 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!';
}
?>
Post Reply