Jump to content

Hey guys,

 

So i have a login system that checks if the user exists if yes, it will log him in. simple..

Now im thinking about how they can abuse some function or something.

Its really crappy code that im writing but it is just to figure stuff out.

 

-Registration section

I always use prepared statements and do not know if i need to do anything else to make it any safer.

For now nothing really gets escaped (like username, firstname, lastname) will this be a problem for SQL injections?

The password gets hashed with md5 just to try, I WILL change this ofcourse. but since it was my first time i kept it simple.

 

-Login part.

Again prepared statements, nothing gets escaped it just gets checked and if everything is fine they will create the session.

 

I shall post the login system code, many things will and can be edited. keep it nice and i would love some feedback.

 

HTML part

<form action="" method="POST" id="loginform">
	<input type="text" name="useremail" placeholder="name@example.com (required)" value="<?php if(isset($_POST['useremail'])){echo htmlentities($_POST['useremail']);}?>"><br>
	<input type="password" name="password" placeholder="password (required)" value=""><br>
	<button type="submit" name="login" value="submit" form="loginform">Login</button>
</form>

In the same file where the HTML code is we have this IF function and will get triggered when the button is clicked.

then it will check of both fields are filled in if not it will say "Both fields needs to be filled in"

Then it will get the Function i've created LoginScript and passes the Email and Password that has been filled in to the Function so he can do the rest.

If it Returns true it will send you to index.php and you are logged in.

<?php 
	if(isset($_POST['login'])){
		if (isset($_POST['useremail']) && ($_POST['password'])){ //Check is both required fields have been filled in.
			if(LoginScript($_POST['useremail'], $_POST['password']) === true){
				header("Location: index.php");
				die();
			}
		}
		else ErrorMessage('Both fields needs to be filled in.');
	}
?>

Function code

<?php
	function LoginScript($useremail, $password){
      	//Hashes the password into a md5 string
		$hashedpassword = md5($password);
      	//Since we will do some work with SQL include the configs.
		require('configs/constant.php');
		require('configs/database.php');
		$db = getDbConnection();
		$stmt = $db->prepare("SELECT u_id, u_email, u_firstname, u_lastname, u_level, u_city, u_password FROM accounts WHERE u_email = ?");
		$stmt->bindParam(1, $useremail);
		$stmt->execute();
		$user = $stmt->fetch(PDO::FETCH_ASSOC);
      	//Checks if the Useremail input matches with a user in the Database
      	//If it does it will check if the password matches
		if ($useremail === $user['u_email']) {
			if ($hashedpassword === $user['u_password']) {
              	//So the Useremail and Password did match, creates a session now.
				$_SESSION['u_id'] = $user['u_id'];
				$_SESSION['u_email'] = $user['u_email'];
				$_SESSION['u_firstname'] = $user['u_firstname'];
				$_SESSION['u_lastname'] = $user['u_lastname'];
				$_SESSION['u_city'] = $user['u_city'];
				$_SESSION['u_level'] = $user['u_level'];
				header("Location: login.php");
				die();
              	//Returns true.
				return true;
			}
          	//Creates a Error with information
			else{ErrorMessage('*Username or password is invalid.');return false;}
		}
      	//Creates a Error with information
		else{ErrorMessage('*Username or password is invalid');return false;}
	}
?>

I've tryed to explain as much as possible for the people who stumble on this page and have no clue what some stuff does.

Username = Useremail - this might be a bit confusing.

This going to add some checks to it but this is the most simple example of it.

 

So if you like to give some feedback or actually answer my question on how i can make this more secure.

That would be awesome!

 

 

Quote or mention me if not feel ignored 

Link to comment
https://linustechtips.com/topic/581202-php-prepared-statements/
Share on other sites

Link to post
Share on other sites

Your prepared statements are on point, don't worry about them. Even though you've said you're waiting and just using MD5, I would suggest using salted hashes right now, so you don't have to implement it later when you have more code to change.

While MD5 is a simple solution, it is extremely insecure due to one problem with hashing. For example, let's pretend we have three users, Dave, Jack, and Laura, and their passwords are different. After hashing:

Spoiler

 

Dave's password (123456): e10adc3949ba59abbe56e057f20f883

Jack's password (password): 5f4dcc3b5aa765d61d8327deb882cf9

Laura's password (letmein): 0d107d09f5bbe40cade3de5c71e9e9b

 

So it seems very secure. Unfortunately, if Dave and Jack both have "password" as their password, this is the result:

Spoiler

 

Dave's password (password): 5f4dcc3b5aa765d61d8327deb882cf9

Jack's password (password): 5f4dcc3b5aa765d61d8327deb882cf9

Laura's password (letmein): 0d107d09f5bbe40cade3de5c71e9e9b

 

If a hacker discovers that the password to one is "password," then he/she will notice that the hash of the other is the same. Then they can simply break into the other account, and given the amount of people with similar passwords, that could allow a hacker to open multiple accounts at once. Also, if they know what common passwords are in hash form, then they can simply look for those and bypass all the security you tried to add. The solution is called a salted hash. This involves generating a random string of characters, and prefixing or suffixing it to the password. This means that each user will have a different hash in the end.

To implement salted hashes is simple. When saving a password, use password_hash(), like this: password_hash("password here", PASSWORD_BCRYPT); This will use the Blowfish cryptographic algorithm, which outputs a string that is exactly 60 characters long (unless the input password exceeds 256 characters, if I recall correctly). If you use this, each time a similar password is hashed, it will look different. Example: "$2y$10$MWTi3P2Wq3FllE3WuWKs7ey1vlQaA64eSXsbk/.ItZmZXNZcpATl". The hash is really three parts: the algorithm ($2y), the cost (encryption recursion) to use when verifying the password ($10), the salt (MWTi3P2Wq3FllE3WuWKs7ey1vlQaA64eSXsbk/), and the hash (ItZmZXNZcpATl). More info here.

Last of all, you need to verify the password every time the user logs in. To do this, use the password_verify() function. This will parse the hash and match the input password against the hash, returning a boolean representing whether the correct password was entered. Use of password_verify: if(password_verify("password entered here", "password hash here")) { run code here if the correct password was entered }.

One more note on the password hash: if you ever add a hashed password created by password_hash into a hardcoded string in your code, use single quotes, like this: '$2y$10$MWTi3P2Wq3FllE3WuWKs7ey1vlQaA64eSXsbk/.ItZmZXNZcpATl', otherwise PHP will treat the $ followed by text as a variable to resolve.

One more thing: when transferring passwords from client to server, ALWAYS use HTTPS, as using HTTP allows middle-man attacks to simply look at the plaintext, unencrypted password, and steal it easily.

˙ǝɯᴉʇ ɹnoʎ ƃuᴉʇsɐʍ ǝɹɐ noʎ 'sᴉɥʇ pɐǝɹ oʇ ƃuᴉʎɹʇ ǝɹɐ noʎ ɟI

Link to comment
https://linustechtips.com/topic/581202-php-prepared-statements/#findComment-7591635
Share on other sites

Link to post
Share on other sites

1 hour ago, dannytech357 said:

Your prepared statements are on point, don't worry about them. Even though you've said you're waiting and just using MD5, I would suggest using salted hashes right now, so you don't have to implement it later when you have more code to change.

While MD5 is a simple solution, it is extremely insecure due to one problem with hashing. For example, let's pretend we have three users, Dave, Jack, and Laura, and their passwords are different. After hashing:

  Reveal hidden contents

 

Dave's password (123456): e10adc3949ba59abbe56e057f20f883

Jack's password (password): 5f4dcc3b5aa765d61d8327deb882cf9

Laura's password (letmein): 0d107d09f5bbe40cade3de5c71e9e9b

 

So it seems very secure. Unfortunately, if Dave and Jack both have "password" as their password, this is the result:

  Reveal hidden contents

 

Dave's password (password): 5f4dcc3b5aa765d61d8327deb882cf9

Jack's password (password): 5f4dcc3b5aa765d61d8327deb882cf9

Laura's password (letmein): 0d107d09f5bbe40cade3de5c71e9e9b

 

If a hacker discovers that the password to one is "password," then he/she will notice that the hash of the other is the same. Then they can simply break into the other account, and given the amount of people with similar passwords, that could allow a hacker to open multiple accounts at once. Also, if they know what common passwords are in hash form, then they can simply look for those and bypass all the security you tried to add. The solution is called a salted hash. This involves generating a random string of characters, and prefixing or suffixing it to the password. This means that each user will have a different hash in the end.

To implement salted hashes is simple. When saving a password, use password_hash(), like this: password_hash("password here", PASSWORD_BCRYPT); This will use the Blowfish cryptographic algorithm, which outputs a string that is exactly 60 characters long (unless the input password exceeds 256 characters, if I recall correctly). If you use this, each time a similar password is hashed, it will look different. Example: "$2y$10$MWTi3P2Wq3FllE3WuWKs7ey1vlQaA64eSXsbk/.ItZmZXNZcpATl". The hash is really three parts: the algorithm ($2y), the cost (encryption recursion) to use when verifying the password ($10), the salt (MWTi3P2Wq3FllE3WuWKs7ey1vlQaA64eSXsbk/), and the hash (ItZmZXNZcpATl). More info here.

Last of all, you need to verify the password every time the user logs in. To do this, use the password_verify() function. This will parse the hash and match the input password against the hash, returning a boolean representing whether the correct password was entered. Use of password_verify: if(password_verify("password entered here", "password hash here")) { run code here if the correct password was entered }.

One more note on the password hash: if you ever add a hashed password created by password_hash into a hardcoded string in your code, use single quotes, like this: '$2y$10$MWTi3P2Wq3FllE3WuWKs7ey1vlQaA64eSXsbk/.ItZmZXNZcpATl', otherwise PHP will treat the $ followed by text as a variable to resolve.

One more thing: when transferring passwords from client to server, ALWAYS use HTTPS, as using HTTP allows middle-man attacks to simply look at the plaintext, unencrypted password, and steal it easily.

Im aware of the points you have made, still thanks for replying with usefull information hopefully other people will have use for it.

Before i started working on this i already did more than enough research about what to do and glady enough i  can say all the points that you have given were familiar to me.

Seems like i've learned something these days :)

 

Already set a HTTPS connections with A+ grade, but i have lost the keys since i deleted my docker... real smart move.

Probably there is a way i can get them back from letsencrypt.

But before that i will setup a Proxy Nginx to make it nice and clean :)

 

Thanks mate!

Quote or mention me if not feel ignored 

Link to comment
https://linustechtips.com/topic/581202-php-prepared-statements/#findComment-7592008
Share on other sites

Link to post
Share on other sites

6 hours ago, sicet7 said:

I have noticed that there is no CSRF(Cross Site Request Forgery) protection on the given example you might want to look in to that. Alex Garret has made an excellent tutorial on how to do CSRF protection Here.

Thanks, i did hear about CSRF but thought it did not apply to POST.

Thanks for the heads up! will make work of it.

Quote or mention me if not feel ignored 

Link to comment
https://linustechtips.com/topic/581202-php-prepared-statements/#findComment-7629031
Share on other sites

Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

×