3 
    $salt = $this->get_salt($username);

    if (is_null($salt)) {
        return FALSE;
    }

    $password = sha1(SITE_KEY . $password . $salt);
    $sth = $this->db->prepare("SELECT id, username, active FROM user WHERE username = ? AND password = ?");
    $sth->setFetchMode(PDO::FETCH_OBJ);
    $sth->execute(array($username, $password));

    if (($result = $sth->fetch()) !== FALSE) {
        return $result;
    }

    return FALSE;

これが私を心配させるものです：

ログイン方法を誤解していませんでした。そのオブジェクトを返す必要はないと思います。私は間違っているかもしれませんし、あなたがしていることはまったく問題ありませんが、私はそれを疑っています. データベースから完全なユーザー オブジェクト、パスワード、およびすべてを、潜在的に安全でないスクリプトに返しています。誰かが新しいファイルを作成してから、var_dump($userObject); のようなことを行う可能性があります。そして、そのすべての情報を持っています

また、何かから「魔法の」プロパティを返すのは直感的ではありません。これは、使用前に検証されていないものの 1 つにすぎません。それを認証クラスの別のメソッドに移動し、「アクティブ」の値を返すようにすると、login.php スクリプトを賢く使わなくても、必要な検証を実行できます。

: もう一度見てみると、そのオブジェクトをそのように悪用する場合は、ログイン情報を知る必要があります。私の意見では、何らかの漏れがある場合に備えて、それを分離することをお勧めします. 誰かがそれを行う方法を知っていると言っているわけではありません。潜在的な抜け穴が 1 つ少ないと考えてください。

悪意のあるユーザーが何かを行う方法を理解しているふりをしているわけではありません。彼らがそうするのをより簡単にすることは賢明ではないように思われることを私は知っています. 私はセキュリティの専門家ではありません。私は、私がそれを書くとしたら、私にとって危険に見えるので、私が変更するであろうことを突っついている. 危険かどうかを本当に知りたい場合は、その例を通常のスタックオーバーフロー コミュニティに送信して、彼らの意見を確認することをお勧めします。

彼は彼の言っていることのいくつかのポイントを持っていますか? このメソッドは、私が持っているページ コントローラーで使用されます。

$user = $auth->login('username, password)

if ($user) {
//do some additional checks... set session variables
}
4

2 に答える 2

8

いいえ、提供されたコードは安全ではありません。

まず、反復されていない単純なハッシュを使用しています。理由については、この回答とこのブログ投稿を参照してください。

第二に、なぜ塩を別々に保管しているのですか? ソルトが機能するには、すべてのレコードに対して一意である必要があります。したがって、それを配置する論理的な場所は、パスワードの横にあります。この場合、1 つではなく 2 つのクエリを作成しています (ソルトはユーザー名から派生したものではなく、実際にはランダムであると仮定します)。ランダムでない場合は、ランダムにする必要があります。詳細については、この投稿を参照してください...

第 3 に、データベースはタイミングの影響を受けない比較を使用していないため、パスワード ハッシュに対するタイミング攻撃にさらされます。詳細については、この記事とこの記事を参照してください。

自分でこれをしないでください。安全に行うのは本当に難しいです。ライブラリを使用します。PHPASSまたはPHP-PasswordLibを使用できます...

編集：あなたの友人からのコメントへ：

ログイン方法を誤解していませんでした。そのオブジェクトを返す必要はないと思います。私は間違っているかもしれませんし、あなたがしていることはまったく問題ありませんが、私はそれを疑っています. データベースから完全なユーザー オブジェクト、パスワード、およびすべてを、潜在的に安全でないスクリプトに返しています。誰かが新しいファイルを作成してから、var_dump($userObject); のようなことを行う可能性があります。そして、そのすべての情報を持っています

それは有効な点です。しかし、その時点で、ユーザーが を注入できれば、var_dump($userObject)実行して、自分のユーザー データだけでなく、すべて$this->db->prepare("SELECT * FROM user");のユーザー データを取得できます。したがって、指摘すべきことではありますが、保護しようとする価値はありません。あなたが本当に肛門になり、それをすべてデータベースのストアドプロシージャに入れ、テーブルからの読み取りアクセスを制限しない限り（したがって、ユーザーテーブルからはまったく選択できません）。しかし、その時点で、アプリケーション全体をゼロから再設計する必要があります。SP 以外ではユーザー情報を取得できないからです。そして、それをうまく行うのは難しいでしょう (複雑さはセキュリティの敵です)。

最終的には、そもそもスクリプト インジェクションから防御する方がはるかに優れていると思います。特に、その種の攻撃 (PHP プロセスでの chroot jail など) を防ぐために必要なばかげた量までサーバーを保護できるプロの管理者がいない場合はなおさらです。

また、何かから「魔法の」プロパティを返すのは直感的ではありません。これは、使用前に検証されていないものの 1 つにすぎません。それを認証クラスの別のメソッドに移動し、「アクティブ」の値を返すようにすると、login.php スクリプトを賢く使わなくても、必要な検証を実行できます。

どんな魔法の属性がありますか? それらのアイテムを直接使用していますか？それとも、それらを使用してユーザーオブジェクトを設定していますか（データの処理方法を知っています）...？そのスニピットだけでは、あなたの友人が指摘している問題はわかりません...

: もう一度見てみると、そのオブジェクトをそのように悪用する場合は、ログイン情報を知る必要があります。私の意見では、何らかの漏れがある場合に備えて、それを分離することをお勧めします. 誰かがそれを行う方法を知っていると言っているわけではありません。潜在的な抜け穴が 1 つ少ないと考えてください。

実際には、上記のタイミング攻撃により、有効なユーザー名を知る必要があるだけです...

悪意のあるユーザーが何かを行う方法を理解しているふりをしているわけではありません。彼らがそうするのをより簡単にすることは賢明ではないように思われることを私は知っています. 私はセキュリティの専門家ではありません。私は、私がそれを書くとしたら、私にとって危険に見えるので、私が変更するであろうことを突っついている. 本当に危険かどうかを知りたい場合は、その例を通常のスタックオーバーフロー コミュニティに送信して、彼らの意見を確認することをお勧めします。

一般的に、これは良いアドバイスです。複雑さはセキュリティの敵であることを覚えておいてください。したがって、より困難にすることで多くの複雑さが追加される場合、新しい脆弱性 (導入されたタイミング攻撃など) が追加される可能性が大幅に高くなります。私は単純さの側で誤ります。複雑な部分 (パスワード ハッシュなど) を処理するために十分にテストされたライブラリがありますが、私が書くコードは単純です。

最強の暗号化アルゴリズムである One Time Padを考えてみましょう。アルゴリズムは非常に単純です。

encrypted = plaintext XOR key

しかし、鍵なしで解読することは不可能です。したがって、アルゴリズムに対するセキュリティは、アルゴリズムではなく、実際には秘密にあります。これはあなたが望むものです。アルゴリズムは公開する必要があり、セキュリティはその秘密性に依存するべきではありません。そうでなければ、あいまいさによるセキュリティになります。これは、ひびが入るまで暖かくファジーな感じを与えます...

于 2012-05-11T19:57:10.403 に答える
1

bcryptを使用してパスワードをハッシュする方法については、この回答を参照してください。ハッシュアルゴリズムの意図的なスローダウンを可能にするため、あなたのものよりも安全です（したがって、誰かがブルートフォースを試みた場合、時間と年の差である1Bではなく、毎秒1Mの反復に制限されます）。

:placeholderまた、(セキュリティとは関係ありません)、名前のないプレースホルダー ( and not )の代わりに、PDO の名前付きプレースホルダーを使用する必要があります?。

さらに、完全なオブジェクトを構築してそれを返したいと思うかもしれません。Userその方が作業が簡単になります (1 つあると仮定すると、そうするべきです!)。

于 2012-05-11T19:35:59.743 に答える