Solo per motivi di headstuff (così avrai un'idea del motivo per cui Fred -ii- ha detto di non usare questo codice) - Lo smonto in qualche modo, in ordine, mentre lo apro (non è uno scavo personale alla persona che pone la domanda, ma solo per dare un'idea del fatto che provare a costruire un'applicazione sicura a metà sullo stack LAMP richiede un po' di attenzione e previdenza... e un cinismo sanguinario unito all'assumere il peggio l'umanità aiuta):
Punto 1
Non è un grosso problema, ma in realtà se stai per iniziare una sessione dovresti iniziare una sessione indipendentemente dal fatto che sia presente $_POST
dati o meno. Probabilmente dovresti richiedere il tuo file di configurazione e avviare la sessione in alto prima di ogni altra cosa.
Non è un errore terminale (poiché non hai la convalida della sessione) - solo strano.
Punto 2
Hai un output in questo file (echo
) quindi deve essere nella root del documento e disponibile nell'albero web.
include("config.php");
Questo non è scritto correttamente, probabilmente dovrebbe essere require_once 'config.php';
(supponendo che sia un file di programma richiesto e non un'inclusione facoltativa che può non riuscire) ma questo è non l'errore. L'errore è che hai il tuo file di configurazione nella radice del tuo documento. Una configurazione errata del server o un semplice errore di battitura in quel file potrebbe, in teoria, consentire l'output sullo schermo in testo normale del contenuto di quel file, rivelando potenzialmente le stringhe di connessione del database (e chissà cos'altro) a world + dog.
I file di configurazione dovrebbero esistere al di fuori dell'albero web o, in mancanza, all'interno di una directory protetta con qualcosa come .htaccess
Deny from all
. Non dovrebbero mai essere accessibili tramite HTTP.
Punto 3
Il mysql
la libreria è obsoleta e non dovrebbe essere utilizzata affatto; MySQLi o PDO sono la strada da percorrere, idealmente con parametri/valori vincolati:
Personalmente l'avrei preso per DOP.
Punti 4 e 5
$password = mysql_real_escape_string(stripslashes(md5($_POST['password'])));
Prima di tutto, l'ordine di questo è sbagliato. Stai eseguendo l'hashing di $_POST['password']
e poi tentando di eseguire gli stripslash - non ci saranno tagli una volta che è stato eseguito l'hashing. Tuttavia, se stai cercando di impedire alle persone di utilizzare barre (o altro) nelle password, devi rimuoverle prima di eseguire l'hashing della stringa.
Avanti md5
non dovrebbe essere utilizzato come algoritmo di hashing delle password, è risultato debole e può essere forzato brutalmente per creare collisioni di stringhe molto più frequentemente di quanto dovrebbe.
Sì, dovresti memorizzare solo hash o "impronte digitali" delle password anziché le password stesse ma, idealmente si desidera eseguire il salt e l'hash (con almeno sha1
) quelle password invece di inserirle semplicemente in un md5()
funzione.
Vedi:http://uk3.php.net/mcrypt per esempio
E fai una ricerca su "password salting hashing" usando il tuo motore di ricerca preferito.
Punto 6
SELECT id FROM $table
WHERE username = '" . $username . "'
and password = '" . $password . "';
Ho aggiunto nel =
che mancava dalla domanda originale, ma comunque non corrispondono a nome utente e password nella tua query ... se qualcuno è riuscito a ottenere un'iniezione SQL nel tuo nome utente, la password non sarebbe mai stata verificata. Immagina:
SELECT user.id
FROM user WHERE user.username = 'fred' OR 1 = 1
-- AND user.password = 'abc123'
È meglio selezionare l'ID utente e l'impronta digitale della password dal database e quindi valutare la password nell'applicazione piuttosto che fidarsi di un controllo della password nel livello del database. Significa anche che puoi utilizzare un algoritmo di hashing e salting dedicato nell'applicazione stessa per convalidare le tue password.
Punto 7
$_SESSION['user'] = $_POST["username"];
Questo sta semplicemente memorizzando il nome utente nella sessione? Questo non dovrebbe in alcun modo essere usato come un "verificatore di accesso", soprattutto perché non c'è (apparentemente) nulla nella tua sessione per impedire dirottamento .
L'ID della sessione potrebbe essere facilmente annusato dal cookie di una sessione live e questo è tutto ciò che sarebbe necessario per "prendere in prestito" il login di qualcun altro. Dovresti almeno tentare di mitigare la possibilità che la sessione venga dirottata associando l'indirizzo IP dell'utente, la stringa UserAgent o qualche altra combinazione di dati relativamente statici con cui puoi confrontare su ogni pagina... ci sono degli svantaggi praticamente in ogni approccio però (soprattutto, come ho scoperto, se hai visitatori che utilizzano AOL) - ma puoi creare un'impronta digitale di sessione probabilmente 99+% efficace per mitigare il dirottamento con pochissime possibilità che la sessione dell'utente venga erroneamente scaricata.
Idealmente potresti anche voler creare un token per la sessione per mitigare CSRF attacchi quando l'utente ha bisogno di eseguire un'azione "privilegiata" sul database (aggiornare i propri dettagli o altro). Il token potrebbe essere un codice assolutamente casuale e univoco memorizzato nel database e/o in un cookie SSL quando l'utente accede (supponendo che l'utente non possa eseguire alcuna azione che aggiorni il database al di fuori di HTTPS in quanto ciò trasmetterebbe semplicemente i dati in chiaro su Internet, il che sarebbe una cattiva idea ).
Il token viene inserito in un campo modulo nascosto per qualsiasi/tutti i moduli e confrontato con il valore memorizzato nel cookie (o nella sessione o nel database) ogni volta che il modulo viene inviato. Ciò garantisce che la persona che invia il modulo abbia almeno una sessione live sul tuo sito web.