Loginsystem bedømmelse:

Tags:    php

Jeg har lige siddet til et lan uden internet eller bøger og siddet og leget lidt med et loginsystem. Tror nok sessions ville have været nemmere, men nu havde jeg ingen inet adgang så kunne ikke lære noget om det. Ville høre om dette loginsystem ville være sikkert nok til en hjemmeside (har skåret body og html fra):

Loginsiden : (lasse.html)

<form action="index.php" method="post">
Brugernavn: <INPUT TYPE=TEXT NAME=inputbruger><br>
Password: <INPUT TYPE=PASSWORD NAME=inputpass><br><br>
<INPUT TYPE=SUBMIT VALUE=send>
</Form>

Scriptet der skal være på alle siderne :

<?
include("dbinfo.php");
if($inputbruger)
{
mysql_connect ("localhost","$username","$password");
mysql_select_db ("$database");
$query = mysql_query("SELECT `medlemmer`.`bruger` FROM medlemmer ORDER BY bruger");

while($r = mysql_fetch_array($query)) {
$user = $r["bruger"];
if($user == $inputbruger){
$bool_right = "true";
$rigtig_bruger = $user;
$query = mysql_query("SELECT `medlemmer`.`pass` FROM medlemmer WHERE bruger= '$rigtig_bruger'");
while($r = mysql_fetch_array($query)) {
$userpass = $r["pass"];
if($userpass <> $inputpass){ //tjekker om password i forvejen er ens med krypteret,da det kunne være henført fra anden side via link og ikke login
$inputpasskryp = md5($inputpass); //hvis ikke så krypter
}
else $inputpasskryp = $inputpass;
if($userpass == $inputpasskryp){ //brugernavn og password er i overensstemmelse, og her kommer htmlsiden så ind.
?>

//INSERT HTML CODE HERE


Sådan ville et link se ud :

<a href='side2.php?inputbruger=<?echo "$inputbruger"?>&inputpass=<?echo"$inputpasskryp"?> '>Hej</a>


//END HTML CODE HERE

<?
}

else
echo "Forkert brugernavn eller adgangskode. <a href='lasse.html'>Tilbage</a>.";
}
}
}
if($bool_right <> "true") echo "Forkert brugernavn eller adgangskode. <a href='lasse.html'>Tilbage</a>.";
}
else
echo "Du er ikke logget ind, du kan logge ind fra siden <a href='lasse.html'>her</a>.";

?>


Skal have ændret det hele så det virker uden Global variabler = on, men det er vidst også den eneste ændring jeg kan se.


Mvh
Lasse
-----------------------------------------------
Sex er som Quake, Singleplayer er godt. Men multiplayer er bare bedre.
-----------------------------------------------



9 svar postet i denne tråd vises herunder
0 indlæg har modtaget i alt 0 karma
Sorter efter stemmer Sorter efter dato
det er list vanskeligt at gennemskue når du bruger register_globals. Desuden er din kode meget lang, og dermed er det flere steder, hvor der kan være huller. Prøv at korte scriptet lidt ned.




Der er en del ting som jeg personligt ville ændre, f.eks. ville jeg bruge sessions. Men uden sessions kan det også lade sig gøre.
Du inkludere "dbinfo.php", men du laver alligevel en manuel forbindelse, den kunne du måske flytte over i "dbinfo" så du ikke skal skrive det hver gang?
Du vælger også ALLE brugere fra din tabel, du kunne måske nøjes med at vælge dem der har det indtastede brugernavn og password?! Det ville spare en masse kode...
Jeg er heller ikke med på hvordan du på efterfølgende sider holder øje med om folk er logget på eller ej...





Mjah.. Du glemmer at bruge mysql_escape_string når du propper user input ind i din sql. Måden du checker om bruger+password er gyldigt, er ret ueffektivt, og ødelægger hele ideen med at have en database i mysql. Noget i den her retning er nok smartere:
Fold kodeboks ind/udKode 


Jeg ved ikke lige hvor smart det er at tillade en md5sum som password direkte. Der er måske nok en mere sikker måde. Problemet er at en sniffer der fanger md5summen, kan indtaste md5summen som password, uden at skulle lave http header tricks, og så ryger en del af ideen med at bruge hashes.

Det er iøvrigt blevet påvist at md5 ikke er sikker til passwords, og den har aldrig været beregnet til det formål. Brug hellere sha1
--
Thus, I conclude



dobbeltpost

[Redigeret d. 23/08-04 12:06:47 af Thomas Christensen]



Mjah.. Du glemmer at bruge mysql_escape_string når du propper user input ind i din sql. Måden du checker om bruger+password er gyldigt, er ret ueffektivt, og ødelægger hele ideen med at have en database i mysql. Noget i den her retning er nok smartere:
Fold kodeboks ind/udKode 


Jeg ved ikke lige hvor smart det er at tillade en md5sum som password direkte. Der er måske nok en mere sikker måde. Problemet er at en sniffer der fanger md5summen, kan indtaste md5summen som password, uden at skulle lave http header tricks, og så ryger en del af ideen med at bruge hashes.

Det er iøvrigt blevet påvist at md5 ikke er sikker til passwords, og den har aldrig været beregnet til det formål. Brug hellere sha1
--
Thus, I conclude

Men opnår vil den bedste sikker ved bruge mere end én form for kryptering/hash :)
mvh.
Thomas Christensen
-------------------------------------------------------
Visual Basic noget for dig?
tjek Visual Basic-Gruppen
http://www.udvikleren.dk/groups/?gid=41



mysql_escape_string skal kun bruges hvis der ikke bruges magic_quotes



Hej Lasse,

Her et mit bud på hvordan du kan "optimere din kode", da jeg ikke programmere i PHP håber jeg at mine ændringer giver meing:

<?
include("dbinfo.php");

if($inputbruger) {
mysql_connect ("localhost","$username","$password");
mysql_select_db ("$database");

//OVERFLØDIGT! -EMG
//$query = mysql_query("SELECT `medlemmer`.`bruger` FROM medlemmer ORDER BY bruger"); - Burg kun ORDER BY hvis du skal sortere eller vise data, det skal du ikke når du laver brugergodkendelse. "spild af resourcer." ;)
//
//while($r = mysql_fetch_array($query)) {
// $user = $r["bruger"];
//
// if($user == $inputbruger) {
//
// $bool_right = "true";
// $rigtig_bruger = $user;
// $query = mysql_query("SELECT `medlemmer`.`pass` FROM medlemmer WHERE bruger= '$rigtig_bruger'");

$query = mysql_query("SELECT `medlemmer`.`pass` FROM medlemmer WHERE bruger= '$inputbruger'"); // Input bruger bruges som søgekriterie.

while($r = mysql_fetch_array($query)) {
$userpass = $r["pass"];

if($userpass <> $inputpass) { //tjekker om password i forvejen er ens med krypteret,da det kunne være henført fra anden side via link og ikke login
$inputpasskryp = md5($inputpass); //hvis ikke så krypter
}
else $inputpasskryp = $inputpass;
if($userpass == $inputpasskryp){ //brugernavn og password er i overensstemmelse, og her kommer htmlsiden så ind.

// Sætter bool_right her. - EMG
$bool_right = "true";

}

else
echo "Forkert brugernavn eller adgangskode. <a href='lasse.html'>Tilbage</a>.";
}
//}
}
if($bool_right <> "true") echo "Forkert brugernavn eller adgangskode. <a href='lasse.html'>Tilbage</a>.";
else
echo "Du er ikke logget ind, du kan logge ind fra siden <a href='lasse.html'>her</a>.";

?>




Men opnår vil den bedste sikker ved bruge mere end én form for kryptering/hash :)
mvh.
Thomas Christensen
-------------------------------------------------------
Visual Basic noget for dig?
tjek Visual Basic-Gruppen
http://www.udvikleren.dk/groups/?gid=41



Eller ved at køre sin loginside via https, så skulle det da i det mindste være lidt sværre at sniffe dit password og brugernavn.



Mange tak for de gode svar, og mange af dem :D

Dette var mit første selvskrevne phpscritp så self var der en masse fejl i det :P Men er nu stadig godt tilfreds af en firsttimer at være. Men kan godt se alle de ændringer som i har lavet giver mest mening, men som sagt sad til et LAN uden internet :P Så alt jeg havde at programmere udfra, var min huskeviden fra en tynd phpbog og en opslagsfil over forskellige mysql kommandoer :D Så vidste ikke helt hoved eller hale. Vidste ikke man kunne halvdelen af de kald i laver til mySQL :P

Men eftersom der bliver sagt at Sessions er bedst skulle jeg måske lære dette?

Der bliver talt om en anden form for kryptering? Hvad er kaldet til dette? altså kaldet ligesom feks md5($variabel).

Mvh
Lasse

-----------------------------------------------
Sex er som Quake, Singleplayer er godt. Men multiplayer er bare bedre.
-----------------------------------------------



t