0

I Followed the tutorial on manual in order to make my login code more secure which mad it not working now after following the tutorial at http://www.w3schools.com/php/func_mysql_real_escape_string.asp its shows the following error

Warning: mysql_num_rows() expects parameter 1 to be resource, boolean given in /home/content/58/9508458/html/pabrowser/checklogin.php on line 32

Warning: Cannot modify header information - headers already sent by (output started at /home/content/58/9508458/html/pabrowser/checklogin.php:32) in /home/content/58/9508458/html/pabrowser/checklogin.php on line 46

and is not logging in

<?php
function check_input($value)
{
// Stripslashes
if (get_magic_quotes_gpc())
  {
  $value = stripslashes($value);
  }
// Quote if not a number
if (!is_numeric($value))
  {
  $value = "'" . mysql_real_escape_string($value) . "'";
  }
return $value;
}

$link = mysql_connect('xxxxxxx');
if (!$link) {
    die('Could not connect: ' . mysql_error());
}
mysql_select_db("brainoidultrafb", $link);

// username and password sent from form 
$myusername=check_input($_POST['myusername']); 
$mypassword=check_input($_POST['mypassword']); 

 $sql="SELECT * FROM logintbl WHERE stu_email='$myusername' and password='$mypassword'";
$result=mysql_query($sql);

// Mysql_num_row is counting table row
$count=mysql_num_rows($result);

// If result matched $myusername and $mypassword, table row must be 1 row
if($count==1){

// Register $myusername, $mypassword and redirect to file "login_success.php"

session_start();
$_SESSION['username'] = $myusername;
$_SESSION['password'] = $mypassword;

header("location:login_success.php");
}
else {
header('Location: http://www.xxxxxx.com/pabrowser/index.php?succmsg1=INVALID.RETRY');
}
?>

any suggestions to make it more secure?

SomeKittens
  • 38,868
  • 19
  • 114
  • 143
Tina
  • 314
  • 3
  • 6
  • 16
  • 5
    w3schools is not a good place to find reliable web design/development info. – SomeKittens Jun 25 '12 at 16:23
  • can you comment which line of your code is line 32 – Alex W Jun 25 '12 at 16:24
  • Your `SELECT` from `logintbl` failed. – Jonathan M Jun 25 '12 at 16:25
  • 3
    The error is because your SQL query is failing. And you are going to get ripped to shreds here for 1) using w3schools and 2) using the `mysql_*()` functions. – WWW Jun 25 '12 at 16:25
  • 5
    w3schools is a terrible resource for web development. (See: http://w3fools.com/) You shouldn't be using the `mysql_` family of functions in new code. They're deprecated and will be removed in future versions of PHP; use PDO or MySQLi instead. Also, having `stripslashes` in your code is usually a bad sign. Code without magic quotes. – Ry- Jun 25 '12 at 16:25
  • line 32 is $count=mysql_num_rows($result); – Tina Jun 25 '12 at 16:32
  • 3
    w3schools is a wrong and misleading site. You shouldn't use it as reference for any sort of language. For PHP, there's the [PHP Manual](http://php.net), for JavaScript, there's [Mozilla Developer Network (or MDN)](https://developer.mozilla.org/). See http://w3fools.com to further understand why you should never use w3schools. – Madara's Ghost Jun 25 '12 at 16:33
  • 1
    @PeeHaa: That's not the real problem here. The headers are already sent only because of the warnings emitted by the MySQL syntax error. – Ry- Jun 25 '12 at 16:33
  • Change... $result=mysql_query($sql); to... $result=mysql_query($sql) or trigger_error(mysql_error()); and then check the error your query is causing. – Amber Jun 25 '12 at 16:25
  • 2
    @Tina , please stop storing passwords in plain-text format – tereško Jun 25 '12 at 16:35
  • @minitech A right. Unformatted errors are hard to read. – PeeHaa Jun 25 '12 at 16:35

1 Answers1

1

The issue is that your check_input function already quotes strings, so interpolate the results without surrounding quotes:

$sql = "SELECT * FROM logintbl
        WHERE stu_email = $myusername AND password = $mypassword";

As for suggestions to make it more secure:

  1. Drop mysql_ and use parametrized queries, as mentioned in the comments. It saves you escaping nonsense that can be easily left out by accident.
  2. Don't store your users' passwords in plaintext inside a database. Hash them, preferably with salt, preferably with unique salt, preferably with a slow hash like bcrypt.
Ry-
  • 218,210
  • 55
  • 464
  • 476
  • http://chat.stackoverflow.com/transcript/message/4256730#4256730 – PeeHaa Jun 25 '12 at 16:47
  • 1
    Shameless plug: There are many good Stack Overflow posts about Hashing. [Here's just one](http://stackoverflow.com/questions/481160/is-bcrypt-a-good-encryption-algorithm-to-use-in-c-where-can-i-find-it/4435888#4435888). – George Stocker Jun 25 '12 at 16:51
  • There's another good one on Security.SE: http://security.stackexchange.com/questions/211/how-to-securely-hash-passwords – Ry- Jun 25 '12 at 16:58