2016-06-28 11 views
0

SQLとPHPを使用してログインシステムを作成しようとしています。私は、ユーザー名、パスワード、認証レベルの3つのフィールドを持つ標準データベースを持っています。何らかの理由で、コードをテストすると、正しい資格情報で正しいアクセスレベルを使用していても、ログインは失敗します。 PHPにアクセスレベルチェッカーを追加する前に、コードは機能しましたが、はログイン失敗エラーを返します。SQL:条件付きの構文エラーとmysql_fetch_objectの悪用

//If there are input validations, redirect back to the login form 
    if($errflag) { 
     $_SESSION['ERRMSG_ARR'] = $errmsg_arr; 
     session_write_close(); 
     header("location: login-test.php"); 
     exit(); 
    } 

    //Create query 
    $qry="SELECT * FROM members WHERE username='$username' AND password='$password'"; 
    $result=mysql_query($qry); 
    $row = mysql_fetch_object($qry); 

    //Check whether the query was successful or not 
    if($result) { 
     if(mysql_num_rows($result) == 1 && $row->authlevel == "admin") { 
      //Login Successful 
      session_regenerate_id(); 
      $member = mysql_fetch_assoc($result); 
      $_SESSION['SESS_MEMBER_ID'] = $member['username']; 
      $_SESSION['SESS_FIRST_NAME'] = $member['firstname']; 
      $_SESSION['SESS_LAST_NAME'] = $member['username']; 
      session_write_close(); 
      header("location: admin_index.php"); 
      exit(); 
     } else { 
      //Login failed 
      header("location: login-failed.php"); 
     } 
    }else { 
     die("Query failed"); 
    } 
?> 
+0

なぜパスワードを探しているのですか?ハッシュされていて、送信されていないのはなぜですか?そのようなユーザー名を見つけるなど、次に比較する。また、mysqlを使用しないでください。また、SQLに変数を入れてください。 – ArtisticPhoenix

+2

''; *あなたのユーザー名のエントリであれば、あなたのログインシステムをハッキングしましたが、ここからauthLevel = "admin" limit 1 - 'を選択してください。ただ言って。 – ArtisticPhoenix

+1

このSQLには非常に多くの間違いがあります。mysqliで適切なログインフォームを作成するためのチュートリアルにリンクすることはできますか? – Ctc

答えて

2

私のコメントで述べたように、PDOまたはmysqliを使用して準備文を使用してください。誰かがちょうど$usernameについて

'; Select * from members where authLevel = "admin" limit 1 -- 

でこれを入れて、管理者としてログイン強打することができます。これは、クエリがなり、--はその後何が無視されるので、コメントのMySQLの道は何かということですなぜ

SELECT * FROM members WHERE username=''; Select * from members where authLevel = "admin" limit 1 -- AND password='ababsdf' 

です。基本的に私は、authLevelのadminを持つユーザーを選択し、1つの結果に制限するように指示しました。

UPDATE 2可能性がある答えについては、

  • 1 AUTHLEVELは、

  • 2間違っていると可能性が高いあなたが重複したユーザレコードを持っています。だから、行の数はチェックadminまた

を失敗したスペースでAdminまたはADMINまたは[space]adminと同じではありません。 PHPの文字列比較では大文字と小文字が区別されます。

この条件が満たされていないにもかかわらず、そこにある項目の一方または両方が真ではありません。あなたがしなければならないのはこれです。

echo 'NumRows: '.mysql_num_rows($result); 
    echo "<br>\n"; 
    echo 'AuthLevel: '.$row->authlevel; 

これを出力するだけで、かなり明白になるはずです。またAUTHLEVELのためにそのヘッダリダイレクション(あなたが他のページに追い出されません)

 //header("location: login-failed.php"); -- un-comment when fixed. 

をコメントアウトしますが、この

echo 'AuthLevel: ['.$row->authlevel.']'; 

なぜのようなブラケットでそれをラップすることをお勧めしますか?このような場合は

[ admin] 

そこにスペースや何かがあることがわかります。それはあなたの状態でこれを行うには悪い考えではないでしょう

if(strtolower(trim($row->authlevel)) == 'admin' ... 

PDOまたはmysqliのは)(あなたは塩が少なくともSHA256の暗号化を追加(またはpassword_hashを使用追加したいと思い、そのはるかに困難実際にではありません)基本的にはあなたのコードは、PHP7 mysql_のようなセキュリティ上の理由の上にこの

$dsn = 'mysql:host=127.0.0.1;dbname=members;'; 
$user = 'db_user'; 
$password = '*******'; 

try { 
    $DB = new PDO($dsn, $user, $password); 
} catch (PDOException $e) { 
    die('Connection failed: ' . $e->getMessage()); 
} 

$qry="SELECT * FROM members WHERE username=:username"; ///(add field named salt, this is a random string) 
//search only for username 

///$DB is pdo database object 
//prepare the sql, this 2 step process prevent sql injection by using a placeholder :username instead of the variable directly 
$stmt = $DB->prepare($qry); 
//execute the statement with variables 
$stmt->execute(array(':username' => $username)); 
//retrieve the result row as an object. 
$row = $stmt->fetch(PDO::FETCH_OBJ); 
//Check whether the query was successful or not 
if($stmt->rowCount() == 1) { 
    if($row->authlevel == "admin") { //if it's not an admin no need to check password 
     if(sha256($row->salt() . $password) == $row->password){ 
      //Check password in php, db is case insensitive unless its a binary field. 
      //Login Successful (obviously youll want to update the member to account for a better password) 
      ..... 
     }else{ 
      header("location: login-failed.php"); //change for bad password etc. 
     } 
    } else { 
     //Login failed 
     header("location: login-failed.php"); //change for invalid user level (you do not have authorization to view this page ... etc.) 
    } 
}else { 
    die("Query failed"); //change for username not found, or unknown username 
} 

http://php.net/manual/en/pdo.construct.php

http://php.net/manual/en/function.password-hash.php

ようになります。 *関数がなくなったので、それらを使うのに慣れていないのが最良です。

+1

tbhこれは質問に答えることはできませんが、その良い点にもかかわらずです。 – Ctc

+0

合意されましたが、もちろんこれは保護地域の安全にとって重要です。 – Bora

+0

質問に答えることは不可能です、明らかにauthLevelは間違っています。 num_rowsは表面的ですが、その結果は決して起こらないでしょう.2つの可能性があります。 – ArtisticPhoenix