Do I need to sanitize my login value in php

Question

Following is script I used in login page

<?php
//include config
require_once('includes/config.php');

//check if already logged in move to home page
if ($user->is_logged_in()) {
    header('Location: index.php');
}

//process login form if submitted
if (isset($_POST['submit'])) {

    $username = filter_input(INPUT_POST, 'username');
    $password = filter_input(INPUT_POST, 'password');

    if ($user->login($username, $password)) {
        $_SESSION['username'] = $username;
        header('Location: memberpage.php');
        exit;
    } else {
        $error[] = 'Wrong username or password or your account has not been activated.';
    }
}//end if submit
//define page title
$title = 'Login';

//include header template
require('layout/header.php');
?>

Do I need to sanitize these inputs with at least mysql_real_escape_string or can I use this code?

user.php

<?php
include('password.php');
class User extends Password{

    private $_db;

    function __construct($db){
        parent::__construct();

        $this->_db = $db;
    }

    private function get_user_hash($username){  

        try {
            $stmt = $this->_db->prepare('SELECT password FROM members WHERE username = :username AND active="Yes" ');
            $stmt->execute(array('username' => $username));

            $row = $stmt->fetch();
            return $row['password'];

        } catch(PDOException $e) {
            echo '<p class="bg-danger">'.$e->getMessage().'</p>';
        }
    }

    public function login($username,$password){

        $hashed = $this->get_user_hash($username);

        if($this->password_verify($password,$hashed) == 1){

            $_SESSION['loggedin'] = true;
            return true;
        }   
    }

    public function logout(){
        session_destroy();
    }

    public function is_logged_in(){
        if(isset($_SESSION['loggedin']) && $_SESSION['loggedin'] == true){
            return true;
        }       
    }

}


?>

password_verify() code

public function password_verify($password, $hash) {
        if (!function_exists('crypt')) {
            trigger_error("Crypt must be loaded for password_verify to function", E_USER_WARNING);
            return false;
        }
        $ret = crypt($password, $hash);
        if (!is_string($ret) || strlen($ret) != strlen($hash) || strlen($ret) <= 13) {
            return false;
        }

        $status = 0;
        for ($i = 0; $i < strlen($ret); $i++) {
            $status |= (ord($ret[$i]) ^ ord($hash[$i]));
        }

        return $status === 0;
    }

}

Since am new to PHP I am confused about it. Can someone help me?

or will user class will protect this login

Answer 1

Does this actually work?

I see:

if($this->password_verify($password,$hashed) == 1){

But there is no password_verify() method in your class. If you mean the password_verify() function, you should change it to:

if(password_verify($password,$hashed)){

To answer your question, you are using prepared statements so you should not escape your data, you are safe from sql injection. Even if you needed to - if you would inject a value directly into a query somewhere for example - you would need a PDO escaping function and not the mysql_* one. But using prepared statements consistently is obviously the better solution as you cannot forget anything by accident.

The only real problem that I can see is that you echo out your catched exception message to the user. You should log that instead and present the user with a non-technical error message.

And of course you should add error handling to the database calls: An empty result is not an error and will not lead to an exception, but it will lead to problems in your code, so you should check for that.