Domanda

<?php
class test_class {

        public function __construct() { 

        }
        public function doLogin($username,$password) {

            include("connection.php");

            $query = "SELECT *
                      FROM users
                      WHERE username = '".mysql_escape_string($username)."'
                      AND password = '".mysql_escape_string($password)."'";
            $result = mysql_fetch_array(mysql_query($query));
            if(!$result) {

            return 'no';
            }
            else 
                {
            return 'yes';
                }
            }


}
?>

Il codice sopra funziona, ma un po 'preoccupato se la sua sicura o no.

Nota: non sto usando il metodo POST, quindi devo riceverlo come argomenti nella funzione e non posso usare.

if (isset($_POST['username']) && isset($_POST['password']))
        {
        $username= $_POST['username'];
        $password= $_POST['password'];
È stato utile?

Soluzione

Il codice potrebbe essere sicuro, ma l'implementazione non è grande. Non si dovrebbe mai memorizzare una password di autenticazione in testo semplice. Si dovrebbe sale e hash.

ho potuto trascorrere un'ora spiegare perché, ma Faresti meglio solo leggendo questo .

Altri suggerimenti

La query stessa appare sicuro, ma se è stato utilizzato un'interfaccia DB che ha sostenuto vincolante parametro, come DOP o Zend_Db, non avrebbe dovuto scrutare ogni istruzione SQL abbastanza così nervosamente.

Inoltre, le funzioni per MySQL * sono praticamente deprecato; si dovrebbe guardare le funzioni mysqli- * invece.

Come nota a margine stilistico, non ha senso in un costruttore vuoto, e io suggerirei di tornare booleano vero o falso, piuttosto che i valori di stringa.

Infine, come già altrove, la memorizzazione di password in chiaro è una cattiva idea.

uhh .... Sei la memorizzazione di una password in chiaro? Questo è certamente non sicuro. La password deve essere generato un hash con un sale usando qualcosa come sha256. Memorizzazione di password in chiaro non è mai una buona idea.

No. Non si dovrebbe essere memorizzazione della password grezzo nel database. Conservarlo hash (preferibilmente con un sale). Inoltre, le istruzioni preparate sono una scelta migliore di fuggire. Vedere questo PHP DOP documentazione . Come ulteriore vantaggio (oltre la sicurezza), che possono essere più efficienti.

Il codice stesso sembra ok, ma il problema principale che vedo è che si sta passando le password in giro in testo normale.

E 'la connessione client-to-server sicuro (cioè utilizza SSL) È la connessione server-to-database sicuro

Se in entrambi i casi qualcuno può sedersi sul filo e guardare il traffico che passa, allora hai un problema di sicurezza.

Se si trattasse di me, mi piacerebbe sicuramente avere una connessione SSL tra il client e server.

Mi piacerebbe assicurarsi che si erano archiviare un hash della password nel database.

E mi piacerebbe modificare il codice a qualcosa di simile

//Pseduo Code
SELECT * FROM Table where UserName = $username
Get Row Back
if(MD5Hash($password) == DataRow[Password])
   //Valid

Credo che sia OK, però, se sono preoccupati per la sicurezza, mi piacerebbe memorizzare la password di "username" nella variabile e confrontarlo al di fuori della query.

Autorizzato sotto: CC-BY-SA insieme a attribuzione
Non affiliato a StackOverflow
scroll top