User registration and login

I am doubtful about the security of my PHP code. I am new to programming, but want to learn how to secure things, protect my databases from SQL injection, and other best practices. I'd like to know if my databases are safe from attacks with this code.

public function __construct( $data = array() ) {
               if( isset( $data['username'] ) ) $this->username = stripslashes( strip_tags( $data['username'] ) );
              if( isset( $data['password'] ) ) $this->password = stripslashes( strip_tags( $data['password'] ) );
}

I would appreciate any suggestions to existing scripts if mine is insufficient.
If somebody wants to inspect the entire code of this class, here it is:

```
username = stripslashes( strip_tags( $data['username'] ) );
if( isset( $data['password'] ) ) $this->password = stripslashes( strip_tags( $data['password'] ) );
}

public function storeFormValues( $params ) {
//store the parameters
$this->__construct( $params );
}

public function userLogin() {
$success = false;
try{
$con = new PDO( DB_DSN, DB_USERNAME, DB_PASSWORD );
$con->setAttribute( PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION );
$sql = "SELECT * FROM users WHERE username = :username AND password = :password LIMIT 1";

$stmt = $con->prepare( $sql );
$stmt->bindValue( "username", $this->username, PDO::PARAM_STR );
$stmt->bindValue( "password", hash("sha256", $this->password . $this->salt), PDO::PARAM_STR );
$stmt->execute();

$valid = $stmt->fetchColumn();

if( $valid ) {
$success = true;
}

$con = null;
return $success;
}catch (PDOException $e) {
echo $e->getMessage();
return $success;
}
}

public function register() {
$correct = false;
try {
$con = new PDO( DB_DSN, DB_USERNAME, DB_PASSWORD );
$con->setAttribute( PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION );
$sql = "INSERT INTO users(username, password) VALUES(:username, :password)";

$stmt = $con->prepare( $sql );
$stmt->bindValue( "username", $this->username, PDO::PARAM_STR );
$stmt->bindValue( "password", hash("sha256", $this-

Let us review!

• Why are you strip_slashing and strip_tagging the username and passwords before they enter the database? Why do you care? (You don't, I'll get to that in the end).

• Calling __construct() inside of another method of the same object is not recommended. (What does __construct() do? As opposed to what do setUser() and setPassword() do)

• Hashing your password:

• With your salt constant, it loses its meaning. An attacker with access to your server can see your code, and with access to the database, he can easily run a rainbow table against your password database.

• Hashing just with SHA1 once isn't very helpful. SHA1 is too fast and is susceptible to brute force attacks.

• The solution to both is to use password_hash() and password_verify() look them up. (Both of those functions require PHP 5.5 or higher. If you don't have PHP 5.5 or higher, you should use the password_compat library by ircmaxell, who is the same person who wrote the above functions for PHP's core.

• Indentation - Please, for the love of God.

What's wrong with your sanitizing

Sanitizing is an important job of the server. It makes sure nothing nasty comes in and affects what your users are seeing/experiencing.

Sanitizing is done as late as possible, for a simple reason. When the username is entered to the database, you shouldn't make assumptions on who is going to use that username. It could be your application later on, it could be an API you may or may not choose to provide one day, it could be a desktop application or a mobile application which does not care for HTML formatting.

When you extract that value from the database and are about to put it in HTML, that's the point where you should escape for HTML using htmlspecialchars(). Maybe at a different point you may want to output it to a JSON response for an API, in which case, json_encode is good, but htmlspecialchars() not as much.

StackExchange Code Review Q#56690, answer score: 12