Is this piece of code safe - PHP && MySQL
<?php
class test_class {
public function __construct() {
}
public function doLogin($username,$password) {
include("connection.php");
$query = "SELECT *
FROM users
WHERE username = '".mysql_escape_string($username)."'
AND password = '".mysql_escape_string($password)."'";
$result = mysql_fetch_array(mysql_query($query));
if(!$result) {
return 'no';
}
else
{
return 'yes';
}
}
}
?>
The above code works, but is a little concerned about how secure it is or not.
Note. I am not using the POST method, so I have to accept it as arguments in a function and I cannot use.
if (isset($_POST['username']) && isset($_POST['password']))
{
$username= $_POST['username'];
$password= $_POST['password'];
a source to share
The code may be safe, but the implementation is small. You should never store the authentication password in clear text. You have to salt and hash it.
I could spend an hour explaining why, but you'd better just read this .
a source to share
The query itself appears to be safe, but if you were using a DB interface that supports parameter binding, such as PDO or Zend_Db, you wouldn't have to parse every SQL statement so nervously.
Also, the mysql- * functions are pretty outdated; you should look at mysqli- * functions instead.
As a stylistic side of the note, there is no point in an empty constructor and I suggest returning boolean true or false values rather than string values.
Finally, as mentioned elsewhere, storing passwords in cleartext is a bad idea.
a source to share
No. You don't have to store a raw password in your database. Store hashed (preferably with salt). Plus, prepared statements are a better choice than escaping. See PHP PDO documentation . As an added benefit (beyond security), they can be more efficient.
a source to share
The code itself looks ok, but the main problem I see is that you are passing passwords in plain text.
Is the client-to-server connection secure (eg using SSL) Is the connection between the server and the server secure?
If anyone can sit on the wire and monitor traffic anyway, then you have a security issue.
If it were me, I would definitely have an SSL connection between the client and the server.
I would make sure you store the password hash in the database.
And I would change the code to something like
//Pseduo Code
SELECT * FROM Table where UserName = $username
Get Row Back
if(MD5Hash($password) == DataRow[Password])
//Valid
a source to share