MY SQL error

[princeofpersia]

Hi guys, im working on a login page and keep getting an error, I have checked my db.php details and they are correct and also end up calling my hosting company to check details and all was right. I have included the link to db.php and also copied and pasted it in the same page but still getting the same result. Table name and field names are correct.

 

this is the error i get and i appreciate if you help me to see what im doing wrong. Thanks in advance.

Query failed: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'Resource id #5' at line 1

Resource id #5

 

	if (isset($_POST['login'])) {

		$email= htmlentities(trim($_POST['email']));
		$password= htmlentities(trim($_POST['password']));
		if (!$email || !$password) echo"<div class='msgerror'>Please enter both Username and Password</div>";
		else {
		$selectemail=sprintf("SELECT * FROM users WHERE myemail='%s'",mysql_real_escape_string($email));
		$myselect=mysql_query($selectemail);

		$result = MYSQL_QUERY($myselect) or die('Query failed: ' . mysql_error() . "<br />\n$myselect"); 
		if (mysql_num_rows($myselect)==0) {

		echo"<div class='msgerror'>This email address does not exist in our database</div>";	
		}

		else {
		$password=md5($password);
		while ($get_row=mysql_fetch_array($myselect)) {
			$myemail=$get_row['myemail'];
			$mypassword=$get_row['mypassword'];
		}

		if($password!=$mypassword) echo"<div class='msgerror'>Wrong username/password. Please try again</div>";
		else {
		$_SESSION['email']=$myemail;
		//header('Location: ../admin/admin.php');
		}}}}
		?>

 

[Muddy_Funster]

You're double running the query...

$myselect=mysql_query($selectemail);
$result = MYSQL_QUERY($myselect) or die('Query failed: ' . mysql_error() . "<br />\n$myselect"); 

[blacknight]

even so hes tryen to pull the data from the table query not the result ...

$result = MYSQL_QUERY($myselect) or die('Query failed: ' . mysql_error() . "<br />\n$myselect"); 			if (mysql_num_rows($myselect)==0) {

should be

$result = MYSQL_QUERY($myselect) or die('Query failed: ' . mysql_error() . "<br />\n$myselect"); 			if (mysql_num_rows($result)==0) {

and

while ($get_row=mysql_fetch_array($myselect)) {

should be

while ($get_row=mysql_fetch_array($result)) {

[princeofpersia]

well done, you are great. it worked fine.

 

Thank you so much

[Psycho]

You have some other problems in your code.

 

1. I'd advise against using a false check for the $mail/$password variables. If a user created the password such as '000000' it would resolve to false. Instead use empty().

 

2. Why are you using htmlentities() on the password? You should never be displaying it. Also, I would not use htmlentities() on the stored username. It can create problems with validations and you have to make your DB field larger than what you set as a max length for the username. Personally, I store data as the user entered it and then make any transformations based upon the respective output. Also, you don't need to trim() the password as a user may want to actually use spaces before/after other content. It won't fail your script, but it makes their password less secure since a password entered as "___password___" would become just "password"

 

3. You should only be displaying database errors in a development environment - never in a production environment. Just implement some logic to display different errors based upon the environment.

 

4. Try to avoid using '*' in your select queries. It creates unnecessary overhead (i.e. performance). Just select the fields you are going to use.

 

5. I would also not use a separate check for the username and one for password. That gives a malicious user too much information. Simply check both the username and password for a match. If neither matches tell the user that theri information could not be authenticated. As an example, let's say someone wanted to crack into bob@domain.com's accounts on any sites he uses. If the user types that username into your page above, the user would know that bob does have an account on your page. He then only needs to try and hack the password. If you simply stated that there was no match on the username/password combination the malicious user wouldn't even know if bob has an account on your site or if he used a different email address.

 

6. You have a while() loop to check the DB results. If these are user account's wouldn't you only have one matching record?

 

7. Lastly, use indentations to give your code a logical structure to make it easier to "see" the logic flow.

 

Here is a revision of your code above to use as you see fit

if (isset($_POST['login']))
{
    //Pre-process post data
    $email    = isset($_POST['email']) ? trim($_POST['email']) : '';
    $password = isset($_POST['password']) ? $_POST['password'] : '';

    if (empty($email) || empty($password))
    {
        echo"<div class='msgerror'>Please enter both Username and Password</div>";
    }
    else
    {
        $query = sprintf("SELECT myemail FROM users WHERE myemail='%s' AND mypassword='%s' LIMIT 1",
                         mysql_real_escape_string($email),
                         md5($password));
        $result = mysql_query($query) or die('Query failed: ' . mysql_error() . "<br />Query: $query\n");

        if (!mysql_num_rows($myselect))
        {
            echo"<div class='msgerror'>Wrong username/password. Please try again.</div>";    
        }
        else
        {
                $_SESSION['email']=$myemail;
                //header('Location: ../admin/admin.php');
        }
    }
}