此PHP代码对于登录系统看起来是否安全？

这只是它的一部分，但它首先进行连接，然后检查用户名是否存在，然后将数据插入表中。我对PHP并不是很了解，所以没有必要对我进行攻击。试图在这里学习，我想知道我是否在正确的轨道上。此PHP代码对于登录系统看起来是否安全？

require("constants.php"); 
try { 
    $DBH = new PDO("mysql:host=$host;dbname=$dbname", $dbconnect, $dbpass); 
    $DBH->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);  
} 
catch(PDOException $e) { 
    echo "sorry, something happened. try going back and try again."; 
    file_put_contents('PDOErrors.txt', $e->getMessage(), FILE_APPEND); 
} 

function checkName(){ 
$STH = $DBH->query('SELECT username FROM users WHERE username = $username'); 
$STH->setFetchMode(PDO::FETCH_OBJ); 
while($row = $STH->fetch()) { 
    if($username != $row->username){ 
    $check = 1; 
    } 
    else{ 
    $check = 0; 
    } 
    return $check; 
} 
function createSalt() 
{ 
    $string = md5(uniqid(rand(), true)); 
    return substr($string, 0, 3); 
} 
function register(){ 
$check = checkName(); 
if($check == 1){ 
$salt = createSalt(); 
$hash = sha1($salt . $hash); 
$data = array($username, $hash, $salt, $ip); 
$STH = $DBH->("INSERT INTO users (username, password, salt, ip) values (?, ?, ?)"); 
$STH->execute($data); 
} 
}

来源

2010-12-15 mcbeav

而不是通过数据库中的所有用户循环检查是否采取'用户名'，我只是'SELECT ID FROM用户WHERE用户名= $用户名'，如果返回结果，你知道它已被采取。 – stealthyninja 2010-12-15 05:47:51

关于编写登录系统，“我对PHP的了解不多”，这就是为什么那些新开发的软件不应该编写登录系统的原因。编写登录系统是一个充满安全隐患的话题，只有一位经验丰富的软件开发人员才能真正理解。新程序员不仅要学习编程语言的基础知识，还必须同时理解复杂的安全问题。新程序员应该使用别人写的系统，他们实际上知道自己在做什么（例如Barebones SSO）。 – CubicleSoft 2015-05-05 14:03:22

不，它不安全/写得很好。它假定注册全局变量已启用，并且使用全局变量代替函数参数，这使代码非常难以移植到不同的上下文中。抛开糟糕的格式，完全没有任何评论会使代码的维护变得困难 - 良好的编程风格是良好编程的先决条件 - 因此也是安全性。

代码本身也存在特定的问题。它假定$用户名被引用，将无效运行。而且，由于您将$ username字符串与数据库返回的内容进行了比较，因此显然不能正确转义，这意味着代码对注入攻击是开放的。由于您使用的是PDO，因此解决方案是简单地使用准备好的语句/变量绑定 - 您实际上已经使用INSERT完成了！

遍历每个匹配的用户名没有任何意义，并严重破坏行为。一个更好的办法（但不正确的）将是：

/** 
* @param username string - a candidate username 
* @param DBH - connected PDO object referencing user database 
* @return bool - true if username does not exist already 
* 
* search the current list of users to see if the candidate username is available 
*/ 
function checkName($username, $DBH){  
    $STH = $DBH->prepare('SELECT username FROM users WHERE username = :username'); 
    $STH->execute(array(':username'=>$username)); 
    $STH->setFetchMode(PDO::FETCH_OBJ); 
    $row = $STH->fetch(); 
    if ($row === false) { 
    die('whoops!'); 
    } 
    return $username!==$row->username; 
}

正确的解决方案：假设你的用户名是唯一的（他们真的，真的应该是），则不用检查，如果用户名之前存在INSERT - 在INSERT之后创建唯一的索引并检查重复的键失败。

接下来，您可以在初始连接周围使用try/catch，但在随后的查询中不会检查错误。在发现异常之后，尽管您记录并报告了一个错误，但您似乎并未解决此时的控制流，以防止执行其他代码。

对不起 - 这不是很好的代码，更不用说安全。

来源

2010-12-15 11:33:26 symcbean

+1为残酷的诚实赢得胜利。 – 2011-01-27 09:27:26

使用prepared statements，而非内联变量
停止使用全局变量。改为使用$_POST['name']。
if($username != $row->username){什么??！？！由于用户名是唯一的 - 它可以是1个正确的行（总是正确的）或0行。

来源

2010-12-15 05:44:11 zerkms

我喜欢你如何说不使用全局变量，然后立即建议**超级**全局。当然，你绝对是对的，但我喜欢并列。 – AgentConundrum 2010-12-15 05:48:51

@AgentConundrum：hehe ;-) Php不提供任何其他方式来访问请求数据。如果php有任何入口点和请求对象 - 我会建议他们然后;-) – zerkms 2010-12-15 06:18:03

我知道，我只是觉得它很有趣。我确实说过你是绝对正确:) – AgentConundrum 2010-12-15 06:29:47

你不定义$的用户名，并使用它在一个单引号字符，所以没有办法SELECT查询将执行
您应该使用准备好的语句和绑定用户名到SELECT查询中的参数，而不是直接将用户名直接粘贴到字符串中
您不需要选择具有该用户名的用户以查看它是否已被使用，您只需选择计数并查看如果它不为零
如果你的SELECT查询返回任何行，你checkName函数不返回任何价值，因为你只能通过行

我希望这些意见是有用的内环路返回。

来源

2010-12-15 05:44:45

此PHP代码对于登录系统看起来是否安全？

回答

相关问题