此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代碼對於登錄系統看起來是否安全？

回答

相關問題