My first foray into PDO, question on data santitization

[schwim]

Hi there everyone!

I'm following this dandy PDO resource in an effort to write my first script that doesn't use mysqli_query because it's been said not using PDO is for suckers. I'm just getting started and have a long way to go in changing my way of thinking but I've got a question concerning sanitizing user input.

My reading concerning PDO is heavily steeped in it's protection against sql injection. It leaves me wondering what sanitization of user input is needed with PDO? I understand there's no single answer due to the wide range of input but as an example, I'm wondering about someone submitting their username and password. Would this be sufficient to protect the db against malicious user input? If not, what else would you suggest be done?

$username = substr(filter_var($_POST['username'], FILTER_SANITIZE_STRING), 0, 50);

I feel lost without my real_escape_strings 🙂

[schwim]

It's quiet. Almost too quiet.

[Weedpacket]

Did you just bump a thread in less than a day?

Anyway, what sanitisation is needed on user data depends on what the user data is for. Your sample, for example, would strip HTML tags from a username, which would probably be appropriate there, but not for something that is supposed to be HTML. And even then, if a username is supposed to be plain text, then it should go through htmlspecialchars before being displayed in an HTML page anyway. (I'm a member on one forum where <foo> and 🐵 are legitimate usernames.)

Of course, none of that has anything to do with PDO, but with your application.

If your concern is around user input breaking SQL queries, that is what using parameterised queries aka prepared statements avoids. The escaping is built in. If you don't use those (and why wouldn't you?) then PDO still has a quote() method (though its manual page still recommends you use prepared statements instead).

[schwim]

Weedpacket Did you just bump a thread in less than a day?

Definitely not. 0.0

Weedpacket If your concern is around user input breaking SQL queries, that is what using parameterised queries aka prepared statements avoids. The escaping is built in. If you don't use those (and why wouldn't you?) then PDO still has a quote() method (though its manual page still recommends you use prepared statements instead).

Here's what I've come up with from reading. Does this cover what you're speaking about? In my case, usernames will have to be alnum.

	$stmt = $pdo1->prepare("SELECT id, username, password FROM fivemin_users 
	WHERE username = :username AND password = :password");

	$stmt->bindParam(':username', $username, PDO::PARAM_STR, 25);
	$stmt->bindParam(':password', $password, PDO::PARAM_STR, 40);
	$stmt->execute();

	$user_id = $stmt->fetchColumn();

[Weedpacket]

Yeah; like that.

[laserlight]

That said, your sample SQL query implies that you might not be using password_hash() and password_verify() :p

If that's really the case, then what you should be doing instead is to select the id, username, and hashed password where the username matches the username supplied; the password supplied is not part of this query. Then, you call password_verify() to see if the hashed password retrieved matches the password supplied (i.e., password_verify will handle the hashing of the password supplied with the salt for you).

[schwim]

laserlight (i.e., password_verify will handle the hashing of the password supplied with the salt for you).

Thanks very much for the help!

I've read the man page and some SE discussions and I see a lot of mention that the hash function provides the salt so does that mean I never need to store or retrieve this salt? How does the script or function retain that salt for it's future use? I wonder how moving script and db to another server would work?

My other question(sorry) is how does password_verify differ from simply doing a query for username and hashed password and looking for a result?

		$password = password_hash($password, PASSWORD_DEFAULT);

		/* prepare the select statement */
		$stmt = $pdo1->prepare("SELECT id, username, password FROM fivemin_users 
		WHERE username = :username AND password = :password");

		$stmt->bindParam(':username', $username, PDO::PARAM_STR, 25);
		$stmt->bindParam(':password', $password, PDO::PARAM_STR, 40);
		$stmt->execute();

		$user_id = $stmt->fetchColumn();

		if($user_id == false){

[laserlight]

schwim I've read the man page and some SE discussions and I see a lot of mention that the hash function provides the salt so does that mean I never need to store or retrieve this salt? How does the script or function retain that salt for it's future use? I wonder how moving script and db to another server would work?

The hash produced by password_hash consists of a string to identify the hash algorithm, the salt, and the password hash itself. So when you store it in the database, all the information is retained, even if you move it to another database.

schwim My other question(sorry) is how does password_verify differ from simply doing a query for username and hashed password and looking for a result?

It would be the same thing, if you could compute the hashed password. To do that is easy: you just need to know the hashing algorithm used, the salt, and then from there compute the hash of the password that the user enters. Each user has a different salt, so you would need to query the database for the salt. Since the salt is part of password hash, this means that you have to run a query "to select the id, username, and hashed password where the username matches the username supplied; the password supplied is not part of this query". Great, so now you have the hashing algorithm and the salt. So, you take the password the user entered and compute the hash. You are about to run another "query for username and hashed password and looking for a result"... but that's silly. What else do you want from the database? You already have the hashed password from the database, so you just need to check for the match. Sure, you could get the database to do that for you, but then do you always make a database query whenever you want to check if two PHP strings compare equal?

What you did in your example was to compute a hash of the user's password with an entirely new salt (and possibly a different algorithm). That's great, but even with the same password, it is highly unlikely that there is any entry in the database with the same username and hashed password because for that to happen, the salt generated by password_hash has to be exactly the same as the salt generated by password_hash previously, and since the salt is randomly generated, that is like winning the lottery (maybe like winning it several times).

[schwim]

Thank you so much for your continued assistance!

I have managed to create a working system using password_hash and password_verify. I was wondering, however, if you wouldn't mind taking a look at it and letting me know if I've still managed to utilize unsafe elements:

	if(empty($notice)){

		//$password = password_hash($password, PASSWORD_DEFAULT);
		
		/* prepare the select statement */
		$stmt = $pdo1->prepare("SELECT id, user_id, username, password FROM fivemin_users 
		WHERE username = :username LIMIT 1");

		$stmt->bindParam(':username', $username, PDO::PARAM_STR, 25);
		$stmt->execute();
		$row = $stmt->fetch(PDO::FETCH_ASSOC);
		$hpass = $row['password'];
		$user_id = $row['user_id'];

		if($row == false OR (!password_verify($password, $row['password']))){
			
			$notice[] = array(
				"lvl"=>"danger", 
				"msg"=>"Bad news. That login failed."
			);
		
		}else{
			
			/* Successful login. Update user table, set cookie and redirect. */
			
			$identifier = $row['user_id'];
			$authkey = md5(uniqid(rand(), true));
			
			if(ISSET($_POST['remember'])){
				$timeout = time() + 60 * 60 * 24 * 365;
			}else{
				$timeout = 0;
			}
			
			$sql = "UPDATE `fivemin_users` SET `authkey` = :authkey, `timeout` = :timeout WHERE user_id = :user_id";
			$stmt = $pdo1->prepare($sql);
			$stmt->bindValue(':user_id', $user_id);
			$stmt->bindValue(':authkey', $authkey);
			$stmt->bindValue(':timeout', $timeout);
			$update = $stmt->execute();
			
			setcookie('authentication', "$identifier:$authkey", $timeout);
			$_SESSION['loggedin'] = '1';
			header('Location: ?do=default');
			exit;
		}
	}
	
	/* This page requires notice msgs */
	$notifications = notifications($notice);

[laserlight]

I would suggest moving the code to authenticate the user into a function, e.g.,

function authenticate_user($pdo, $username, $password) {
	$sql = "SELECT id, user_id, password FROM fivemin_users WHERE username = :username LIMIT 1";
	$stmt = $pdo->prepare($sql);
	if (!$stmt) {
		return false;
	}

	$stmt->bindParam(':username', $username, PDO::PARAM_STR, 25);
	if (!$stmt->execute()) {
		return false;
	}

	$row = $stmt->fetch(PDO::FETCH_ASSOC);
	if ($row && password_verify($password, $row['password'])) {
		return $row['user_id'];
	}
	return false;
}

/* ... */

if (empty($notice)) {
	$user_id = authenticate_user($pdo1, $username);
	if ($user_id === false) {
		$notice[] = [
			"lvl" => "danger",
			"msg" => "Bad news. That login failed."
		];
	} else {
		/* Successful login. Update user table, set cookie and redirect. */

		$authkey = md5(uniqid(rand(), true));

		$timeout = isset($_POST['remember']) ? (time() + 60 * 60 * 24 * 365) : 0;

		$sql = "UPDATE `fivemin_users` SET `authkey` = :authkey, `timeout` = :timeout WHERE user_id = :user_id";
		$stmt = $pdo1->prepare($sql);
		$stmt->bindValue(':user_id', $user_id);
		$stmt->bindValue(':authkey', $authkey);
		$stmt->bindValue(':timeout', $timeout);
		$update = $stmt->execute();

		setcookie('authentication', "$user_id:$authkey", $timeout);
		$_SESSION['loggedin'] = '1';
		header('Location: ?do=default');
		exit;
	}
}

/* This page requires notice msgs */
$notifications = notifications($notice);

[Weedpacket]

laserlight

$sql = "SELECT id, user_id, password FROM fivemin_users WHERE username = :username LIMIT 1";

I see you dropped selecting username (since one already had that information it didn't need selecting). I don't see id being used anywhere either. I'm also guessing the LIMIT 1 doesn't add anything either, especially if there is a UNIQUE index on the username field (after all, if usernames were duplicated there would be some pretty awkward problems).