<?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 slightly worried whether its secure or not.
上面的代码有效,但有点担心它是否安全。
Note: I am not using POST method, so i have to receive it as arguments in the function and i cannot use.
注意:我没有使用POST方法,所以我必须在函数中接收它作为参数,我不能使用。
if (isset($_POST['username']) && isset($_POST['password']))
{
$username= $_POST['username'];
$password= $_POST['password'];
6 个解决方案
#1
The code might be secure but the implementation is not great. You should never store an authentication password as plaintext. You should salt and hash it.
代码可能是安全的,但实现并不是很好。您永远不应将身份验证密码存储为纯文本。你应该加盐并散列它。
I could spend an hour explaining why, but you'd do better just reading this.
我可以花一个小时来解释原因,但是你只需阅读本文就可以做得更好。
#2
The query itself appears secure, but if you used a DB interface that supported parameter binding, such as PDO or Zend_Db, you wouldn't have to scrutinise every SQL statement quite so nervously.
查询本身看起来是安全的,但如果您使用支持参数绑定的数据库接口(例如PDO或Zend_Db),则不必非常紧张地仔细检查每个SQL语句。
Also, the mysql-* functions are pretty much deprecated; you should look at the mysqli-* functions instead.
而且,mysql- *函数几乎已被弃用;你应该看一下mysqli- *函数。
As a stylistic side note, there's no point in an empty constructor, and I'd suggest returning boolean true or false rather than string values.
作为一个风格的侧面说明,空构造函数没有意义,我建议返回布尔值true或false而不是字符串值。
Finally, as mentioned elsewhere, storing plaintext passwords is a bad idea.
最后,如其他地方所述,存储明文密码是个坏主意。
#3
uhh.... you're storing a plaintext password? That is most certainly not secure. The password should be hashed with a salt using something like sha256. Storing plaintext passwords is never a good idea.
呃....你要存储明文密码?这当然不安全。密码应使用sha256之类的盐进行哈希处理。存储明文密码绝不是一个好主意。
#4
No. You should not be storing the raw password in your database. Store it hashed (preferably with a salt). Further, prepared statements are a better choice than escaping. See this PHP PDO documentation. As an added benefit (besides security), they can be more efficient.
不可以。您不应该将原始密码存储在数据库中。将其保存(最好加盐)。此外,准备好的陈述是比逃避更好的选择。请参阅此PHP PDO文档。作为额外的好处(除了安全性),它们可以更有效率。
#5
The code itself looks ok, but the main issue I see is that you're passing passwords around in Plain Text.
代码本身看起来不错,但我看到的主要问题是你在纯文本中传递密码。
Is the Client-to-Server connection secure (i.e. using SSL) Is the Server-to-Database connection secure
客户端到服务器连接是否安全(即使用SSL)服务器到数据库的连接是否安全
If in either case someone can sit on the wire and watch traffic going by, then you've got a security problem.
如果在任何一种情况下,有人可以坐在电线上观察流量,那么你就遇到了安全问题。
If it were me, I'd definitely have an SSL connection between the client & server.
如果是我,我肯定在客户端和服务器之间有SSL连接。
I'd make sure you were storing a hash of the password in the database.
我确保你在数据库中存储密码的哈希值。
And I'd change your code to something like
我会将您的代码更改为类似的内容
//Pseduo Code
SELECT * FROM Table where UserName = $username
Get Row Back
if(MD5Hash($password) == DataRow[Password])
//Valid
#6
I think it is OK, however, if I am worried about security, I'd store the password of "username" into variable and compare it outside of query.
我认为没关系,但是,如果我担心安全性,我会将“username”的密码存储到变量中,并在查询之外进行比较。
#1
The code might be secure but the implementation is not great. You should never store an authentication password as plaintext. You should salt and hash it.
代码可能是安全的,但实现并不是很好。您永远不应将身份验证密码存储为纯文本。你应该加盐并散列它。
I could spend an hour explaining why, but you'd do better just reading this.
我可以花一个小时来解释原因,但是你只需阅读本文就可以做得更好。
#2
The query itself appears secure, but if you used a DB interface that supported parameter binding, such as PDO or Zend_Db, you wouldn't have to scrutinise every SQL statement quite so nervously.
查询本身看起来是安全的,但如果您使用支持参数绑定的数据库接口(例如PDO或Zend_Db),则不必非常紧张地仔细检查每个SQL语句。
Also, the mysql-* functions are pretty much deprecated; you should look at the mysqli-* functions instead.
而且,mysql- *函数几乎已被弃用;你应该看一下mysqli- *函数。
As a stylistic side note, there's no point in an empty constructor, and I'd suggest returning boolean true or false rather than string values.
作为一个风格的侧面说明,空构造函数没有意义,我建议返回布尔值true或false而不是字符串值。
Finally, as mentioned elsewhere, storing plaintext passwords is a bad idea.
最后,如其他地方所述,存储明文密码是个坏主意。
#3
uhh.... you're storing a plaintext password? That is most certainly not secure. The password should be hashed with a salt using something like sha256. Storing plaintext passwords is never a good idea.
呃....你要存储明文密码?这当然不安全。密码应使用sha256之类的盐进行哈希处理。存储明文密码绝不是一个好主意。
#4
No. You should not be storing the raw password in your database. Store it hashed (preferably with a salt). Further, prepared statements are a better choice than escaping. See this PHP PDO documentation. As an added benefit (besides security), they can be more efficient.
不可以。您不应该将原始密码存储在数据库中。将其保存(最好加盐)。此外,准备好的陈述是比逃避更好的选择。请参阅此PHP PDO文档。作为额外的好处(除了安全性),它们可以更有效率。
#5
The code itself looks ok, but the main issue I see is that you're passing passwords around in Plain Text.
代码本身看起来不错,但我看到的主要问题是你在纯文本中传递密码。
Is the Client-to-Server connection secure (i.e. using SSL) Is the Server-to-Database connection secure
客户端到服务器连接是否安全(即使用SSL)服务器到数据库的连接是否安全
If in either case someone can sit on the wire and watch traffic going by, then you've got a security problem.
如果在任何一种情况下,有人可以坐在电线上观察流量,那么你就遇到了安全问题。
If it were me, I'd definitely have an SSL connection between the client & server.
如果是我,我肯定在客户端和服务器之间有SSL连接。
I'd make sure you were storing a hash of the password in the database.
我确保你在数据库中存储密码的哈希值。
And I'd change your code to something like
我会将您的代码更改为类似的内容
//Pseduo Code
SELECT * FROM Table where UserName = $username
Get Row Back
if(MD5Hash($password) == DataRow[Password])
//Valid
#6
I think it is OK, however, if I am worried about security, I'd store the password of "username" into variable and compare it outside of query.
我认为没关系,但是,如果我担心安全性,我会将“username”的密码存储到变量中,并在查询之外进行比较。