...

View Full Version : Help with login system turning crazy atm



Krentenbol
01-05-2012, 08:02 PM
I have done this atleast a hundred times before, suddenly I just can't anymore-.-.

Code:


<?php
include ('inc/config.php');
mysql_select_db('inlogsysteem', $con) or die;
$gebruikersnaam = $_REQUEST['gebruikersnaam'];
$wachtwoord = sha1($_REQUEST['wachtwoord']);

$result = mysql_query("SELECT wachtwoord FROM gebruikers where gebruikersnaam = '$gebruikersnaam'") or die (mysql_error());
while ($row = mysql_fetch_array($result))

if ($wachtwoord != $row['wachtwoord']) {
echo "Verkeerd wachtwoord";
}
else {
$sql="SELECT * FROM status WHERE gebruikersnaam='$gebruikersnaam'";
$result2=mysql_query($sql);

// Mysql_num_row is counting table row
$count=mysql_num_rows($result2);
echo $count;
// If result matched $ip and $password, table row must be 1 row

if($count==1){
session_register("gebruikersnaam");
echo "Logged in";
}
else
echo "not logged in";
}
?>

My error:

Warning: mysql_num_rows() expects parameter 1 to be resource, boolean given in C:\xampp\htdocs\Login\login.php on line 18

Old Pedant
01-05-2012, 09:09 PM
When mysql_query *FAILS*, the result is a boolean value of false.

So then when you try to use that false with mysql_num_rows() or course you get that error message.

You omiitted the or die( )... form your mysql_query statement.



$sql="SELECT * FROM status WHERE gebruikersnaam='$gebruikersnaam'";
$result2=mysql_query($sql) or die(...whatever you want...);

As an alternative, you could use

$sql="SELECT * FROM status WHERE gebruikersnaam='$gebruikersnaam'";
$result2=mysql_query($sql);
if ( ! $result2 )
{
.... do something ...
} else {
... your existing code ...
]

Old Pedant
01-05-2012, 09:18 PM
COMMENT: Your code is using a bad practice: You give a *different message* for bad user name than you give for bad password.

For hackers, this makes their job easier. They just try many many user names until they get one that says "bad password" (Verkeerd wachtwoord) and then they keep using that user name while they test many many passwords.

Might I suggest a better alternative?



$gebruikersnaam = mysql_real_escape_string($_REQUEST['gebruikersnaam']);
$wachtwoord = sha1($_REQUEST['wachtwoord']);

$sql = "SELECT 'okay' FROM gebruikers " .
" WHERE gebruikersnaam = '$gebruikersnaam'" .
" AND wachtwoord = '$wachtwoord' ";
$result = mysql_query( $sql ) or die (mysql_error());
if ( mysql_num_rows($result) == 0 )
{
// best would be to redirect them back to the login page
echo "Bad username or password.";
exit;
}
...

Now the hacker might have the user name *OR* the password correct, but unless both are correct he gets the same message, and so much less help to the hacker.

Krentenbol
01-06-2012, 09:48 AM
Thank you, it works now. Also thanks for the advice. A friend of me told you could also put the mysql_real_escape_string() in the query. Same with the sha1 so it would look like this:


$sql = "SELECT * FROM gebruikers WHERE gebruikersnaam = '".mysql_real_escape_string($_POST['gebruikersnaam'])."' AND wachtwoord = '".sha1($_POST['wachtwoord'])."'";


Should be better, this is for the people which also need something related to this.

This can be locked.



EZ Archive Ads Plugin for vBulletin Copyright 2006 Computer Help Forum