2016-04-23 16 views
0

私の質問は基本的に、以下のログインスクリプトを使用してセキュリティ上の大きな脅威があるかどうか、もしあれば、SQLインジェクションやユーザーが正しい情報を入力せずにログインするのを防ぐために何ができるのですか?私はPHPの古いバージョンを使用していますが、私のサイト全体がこの以前のバージョンで構築されていることを理解しています。私は私のコードのセキュリティを向上させる手助けし、任意のSQLインジェクションを防ぐためのこのログインスクリプトは安全ですか?

ログインフォーム

<form method="post" action="login.php?login=login"> 

<input type='text' placeholder="username" class='form-control' name='username' required autofocus/> 
<input type='password' placeholder="password" class='form-control' name='password' required/> 
<input class='btn btn-default btn-block' type='submit' value='Login' class='submit' /> 

</form> 

フォームポスト

<? if($login==login) 
     { 
     $username = clean($_POST[username]); 
     $password = md5($_POST[password]); 
     $date = date("Y-m-d"); 
     $time = date("H:i:s"); 
     $sql = mysql_query("select * from users where username = '$username' AND password = '$password'"); 
     $check = mysql_num_rows($sql); 
     if($check!=1) 
     { 
     echo 'Incorrect username or password.'; 
     echo('<meta http-equiv="refresh" content="3;url=/login" />'); 
     $success = "Failed"; 
     if($content[loginlog]==1) 
     $sqllog = mysql_query("insert into usr_logs(user, ip, time, date, success) values('$username', '$ip', '$time', '$date', '$success')"); 
     } 
     else 
     { 
     $user = mysql_fetch_array($sql); 
     $_SESSION[usr_name] = $user[username]; 
     $_SESSION[usr_level] = $user[level]; 
     $_SESSION[usr_ip] = $ip; 
     $success = "Success"; 
     echo('<meta http-equiv="refresh" content="1;url=/home" />'); 
     if($content[loginlog]==1) 
     $sqllog = mysql_query("insert into usr_logs(user, ip, time, date, success) values('$username', '$ip', '$time', '$date', '$success')"); 
     } 
     } 
     if($login==logout) 
     { 
     session_unset(); 
     session_destroy(); 
     echo 'logged out'; 
     echo('<meta http-equiv="refresh" content="3;url=/login" />'); 
     }?> 

ありがとう。

+7

作業コードの質に関する質問は、通常[codereview](http://codereview.stackexchange.com/help/on-topic)に属しているので、この質問をオフトピックとして閉じるよう投票しています – Quentin

+6

ありますこのコードに関する多くの恐ろしいことと、 'clean'関数(実際にあなたが主に尋ねているものです!)が完全に欠けています。 – Quentin

+0

ここで/ '何が' clean() 'ですか? – jDo

答えて

2

まず、clean()関数とは何ですか?

第2に、MD5暗号化方式を使用しないでください。少なくともsha256を使用してください。これはMD5がセキュリティ上の弱点のためにパスワードをハッシュするのが悪く、速すぎるためです。あなたがより多くのセキュリティを使用したい場合はの塩をsha256と組み合わせるとpassword_hash()関数。そして、hereの理由。

第3に、ユーザー入力をそのままデータベースに渡します。つまり、スクリプトはSQLインジェクションに対して脆弱です。 Prepared Statementsを使用してください。公式文書hereは、それらの使用方法を説明しています。

第四に、あなたはこれらの行を交換する必要があります。

echo('<meta http-equiv="refresh" content="3;url=/login" />'); 
echo('<meta http-equiv="refresh" content="1;url=/home" />'); 

をこれらのいずれかで:

header('Location: /login'); 
header('Location: /home'); 

理由は、メタリフレッシュの使用はワールドワイドウェブコンソーシアムによって推奨されていることです。詳細はhereを参照してください。

最後に、旧式の廃止予定のmysql拡張機能を使用しています。データベースにアクセスするには、mysqliまたはPDO拡張機能を使用する必要があります。

+0

またはPDOが良いでしょう – RiggsFolly

+0

@RiggsFollyはい、あなたは正しいです。私はmysqli、thoを使用することを好む。私は私の答えを編集します。 –

0

コードの最大の問題は、虹のテーブルで数秒で壊れるMD5の使用です。 SQLインジェクションの面では、大きな問題があるようです。

'' mysql_query( "$ username"、 '$ ip'などのusr_logs(ユーザー、ip、時刻、日付、成功) '$ time'、 '$ date'、 '$ success') ");

「$ username = 'drop table users ...'」などと言ってください。良くない。

5ish以降のphpバージョンではこれを処理する必要がありますが、私は個人的には銀行の強盗を信用して私の財布を頼っています。

関連する問題