0

i am having problem with converting my sign in script with pdo
here is my script

if ($_SERVER['REQUEST_METHOD'] == 'POST' && $_POST['form_name'] == 'loginform')
{
   $success_page = $_COOKIE["redirect"];
   $error_page = 'signup_error.php';
   $indexlogcheck = 'home.php';
   $mysql_table = 'users';
   $crypt_pass = md5($_POST['password']);
   $found = false;
   $fullname = '';
   $session_timeout = time()+60*60*24*30;
   try
   {
   $pdo = new PDO('mysql:host=localhost;dbname=blog', 'Avik', '');
   }
   catch (PDOException $e)
   {
    $output = 'Unable to connect to the database server.';
    echo $output;
    exit();
    }   

  try
  {
   $sql = "SELECT password, fullname, active FROM ".$mysql_table." WHERE username = '".$_POST['username']."'";
   $statement = $pdo->prepare($sql);
   $statement->execute();
   $result = $statement->fetchAll(PDO::FETCH_ASSOC);
    if ($data = count($result) == 1)
    {
      if ($crypt_pass == $data['password'] && $data['active'] != 0)
      {
         $found = true;
         $fullname = $data['fullname'];
      }
   }
if($found == false)
   {
      header('Location: '.$error_page);
      exit;
   }
   else
   {
      if (session_id() == "")
      {
         session_start();
      }
      $_SESSION['username'] = $_POST['username'];
      $_SESSION['fullname'] = $fullname;
      $_SESSION['expires_by'] = time() + $session_timeout;
      $_SESSION['expires_timeout'] = $session_timeout;
      $rememberme = isset($_POST['rememberme']) ? true : false;
      if ($rememberme)
      {
         setcookie('username', $_POST['username'], time() + 3600*24*30);
      }
      header('Location: '.$success_page);
      exit;
    }
  }
  catch (PDOException $e)
   {
    $output = 'Unable to connect to the database server.';
    echo $output;
    exit();
    } 
}

$username = isset($_COOKIE['username']) ? $_COOKIE['username'] : '';

so where am i doing wrong!!! this script works if i changed it into below codes

try
  {
   $sql = "SELECT password, fullname, active FROM ".$mysql_table." WHERE username = '".$_POST['username']."' AND password = '".$crypt_pass."'";
   $statement = $pdo->prepare($sql);
   $statement->execute();
   $result = $statement->fetchAll(PDO::FETCH_ASSOC);
    if (count($result) == 1)
    {

      if (session_id() == "")
      {
         session_start();
      }

      $_SESSION['username'] = $_POST['username'];
      $_SESSION['fullname'] = $fullname;
      $_SESSION['expires_by'] = time() + $session_timeout;
      $_SESSION['expires_timeout'] = $session_timeout;
      $rememberme = isset($_POST['rememberme']) ? true : false;
      if ($rememberme)
      {
         setcookie('username', $_POST['username'], time() + 3600*24*30);

      }
      header('Location: '.$success_page);
      exit;
   }
    else
    {
    header('Location: '.$error_page);
    exit;
    }
 }

but i want to know whats wrong with the code above!!

4
Contributors
5
Replies
29
Views
3 Years
Discussion Span
Last Post by Eagle.Avik
0

Hi,
line 28 $data=count($result)
But later you use this variable like an array $data['username'] ...
I think this is wrong.

0

There's a few things going on within your code that needs to be addressed. But let's first cover why you're having the log in problem. You've assigned the variable $data to the returned result of the count() function. The result will be an integer, not an associative array as you have try to manipulate it as within your IF statement. You need to use the information contained within the $result variable instead.

The first correction to your code is that your use of a prepared statement is nugatory, since you aren't actually binding anything to your query. You've directly inserted the, potentially tainted, $_POST['username'] value into your SQL. In order to take advantage of the security provided by parametrised queries, you must use a placeholder and either explicity bind the variable to the query with the bindParam() method, or implcitly bind the variable by passing an array to the execute() method.

Another improvement would be to use a different crypotgraphic hashing algorithm, since MD5 isn't really considered "safe" anymore.

Edited by tpunt

0

@tpunt thnx for mentioning the holes in my code, i updated the bind method
here is my updated code

if ($_SERVER['REQUEST_METHOD'] == 'POST' && $_POST['form_name'] == 'loginform')
{
   $redirect_page = $_COOKIE["redirect"];
   $error_page = 'signup_error.php';
   $indexlogcheck = 'home.php';
   $mysql_table = 'users';
   $found = false;
   $fullname = 'avik';
   $session_timeout = time()+60*60*24*30;
   try
   {
   $pdo = new PDO('mysql:host=localhost;dbname=blog', 'Avik', '');
   }
   catch (PDOException $e)
   {
    $output = 'Unable to connect to the database server.';
    echo $output;
    exit();
    }   

  try
  {
   $sql = "SELECT password, fullname, active FROM ".$mysql_table." WHERE username = :username AND password = :password";
   $statement = $pdo->prepare($sql);
   $statement->bindValue(':username', $_POST['username']);
   $statement->bindValue(':password', md5($_POST['password']));
   $statement->execute();
   $result = $statement->fetchAll(PDO::FETCH_ASSOC);
    if (count($result) == 1)
    {

      if (session_id() == "")
      {
         session_start();
      }

      $_SESSION['username'] = $_POST['username'];
      $_SESSION['fullname'] = $fullname;
      $_SESSION['expires_by'] = time() + $session_timeout;
      $_SESSION['expires_timeout'] = $session_timeout;
      $rememberme = isset($_POST['rememberme']) ? true : false;
      if ($rememberme)
      {
         setcookie('username', $_POST['username'], time() + 3600*24*30);

      }
      header('Location: '.$redirect_page);
      exit;
   }
    else
    {
    header('Location: '.$error_page);
    exit;
    }
 }
   catch (PDOException $e)
   {
    $output = 'Unable to connect to the database server.';
    echo $output;
    exit();
    }

}

is this ok, i mean is it now safe from sql injection??

0

and about cryptographic hash algorithm, can anyone give me a link for any tutorial of this!!

This topic has been dead for over six months. Start a new discussion instead.
Have something to contribute to this discussion? Please be thoughtful, detailed and courteous, and be sure to adhere to our posting rules.