[PHP/MySQL] Is mijn inlogsysteem veilig?

[X_lawl_X]

Hallo allemaal,

Ik ben al een tijdje bezig met een site met een inlogsysteem. Ook werk ik met rechten (Iemand met admin-rechten kan meer dingen doen dan iemand met mod-rechten).

Heel het inlogsysteem werkt nu, alleen omdat ik nog nooit een inlogsysteem gemaakt hebt, weet ik niet zeker of dat het wel veilig is. Wat denken jullie?


Dit is het inlogscript:

PHP:

<?php

    require('config.php');

    //controleren of dat er al ingelogd is

    if (isset($_SESSION['password']) && $_GET['q'] != "logout") {
        
        //terug naar de homepage
        header ("location: ucp.php");

        exit;

    }

    

    session_start();

    

    //Inloggen

    if ($_GET['ver'] == "true") {

        $username = mysql_real_escape_string($_POST['username']);

        $password = md5($_POST['password']);

                

        $query = mysql_query("SELECT * FROM Users WHERE username='".$username."' AND password='".$password."'");

        if (mysql_num_rows($query) == "1") {

        

            //Access level toekennen.

            while($row = mysql_fetch_array($query)) {

                $_SESSION['uid'] = $row['uid'];

                $_SESSION['acl'] = $row['acl'];
                
                $_SESSION['username'] = $row['username'];

            }

            
            $_SESSION['password'] = $password;

            $_SESSION['ua'] = $_SERVER['HTTP_USER_AGENT'];

            header("location: ucp.php");

            //indien gewenst, cookies instellen
            if (isset($_POST['setcookie'])) {
                setcookie("username", "".$_SESSION['username']."", time()+60*60*24*31);
                setcookie("password", "".$_SESSION['password']."", time()+60*60*24*31);
                setcookie("uid", "".$_SESSION['uid']."", time()+60*60*24*31);
            }

        } else {

            //wachtwoord of username fout?

            $msg = "<p>Het wachtwoord of de gebruikersnaam is ongeldig.</p>";

        }

            

    }

    

    //uitloggen

    if ($_GET['q'] == "logout") {

        session_destroy();
        
        // cookies laten verlopen als ze bestaan
        setcookie("uid", "", time()-3600);
        setcookie("username", "", time()-3600);
        setcookie("password", "", time()-3600);
        
        $msg = "<p>U bent uitgelogd.</p>";

    }

?>


<?php echo $msg; ?>

<form action="ucp.php?q=login&ver=true" method="post">
<p>Gebruikersnaam:<br />
<input type="text" name="username"></p>
<p>Wachtwoord:<br />
<input name="password" type="password" value="" /></p>
<p><input type="checkbox" name="setcookie" /> Ingelogd blijven?</p>
<input type="submit" class="alinea" value="Log In">
</form>

Op iedere pagina require ik dit script:

PHP:

<?php

    require('config.php');

    session_start();
    
    //sessie ID vernieuwen, iedere keer als er een pagina wordt bekeken.
    
    session_regenerate_id();
    
    //controleren of dat we ingelogd zijn

    if (isset($_SESSION['password']) OR isset($_SESSION['username']) OR isset($_SESSION['uid'])) {

    

        //controleren of dat de login nog steeds klopt.

        $query = mysql_query("SELECT * FROM Users WHERE username='".$_SESSION['username']."' AND password='".$_SESSION['password']."' AND uid='".$_SESSION['uid']."' AND acl='".$_SESSION['acl']."'");

        if (mysql_num_rows($query) != "1") {

            session_destroy();

            die('#1 Sessie mismatch - Log opnieuw in');

        }

        

        //kijken of de User Agent nog steeds overeen komt.

        if ($_SERVER['HTTP_USER_AGENT'] != $_SESSION['ua']) {

            session_destroy();

            die('#2 Sessie mismatch - Log opnieuw in');

        }

        



    } 
    
    elseif (isset($_COOKIE['username'])) {
        $username = mysql_real_escape_string($_COOKIE['username']);
        $md5pass = mysql_real_escape_string($_COOKIE['password']);
        $uid = mysql_real_escape_string($_COOKIE['uid']);
        
        $query = mysql_query("SELECT * FROM Users WHERE username='".$username."' AND password='".$md5pass."' AND uid='".$uid."'");
        
        if (mysql_num_rows($query) == "1") {
            while($row = mysql_fetch_array($query)) {

                $_SESSION['uid'] = $row['uid'];
                $_SESSION['username'] = $row['username'];
                $_SESSION['password'] = $row['password'];
                $_SESSION['acl'] = $row['acl'];
                $_SESSION['ua'] = $_SERVER['HTTP_USER_AGENT'];
                
            }
        
        } else {
            // cookies laten verlopen als ze bestaan
            setcookie("uid", "", time()-3600);
            setcookie("username", "", time()-3600);
            setcookie("password", "", time()-3600);
        }
    }
    else {
        //We hebben te maken met een gast
        $_SESSION['acl'] = "3";
    }

?>

Om te kijken of een gebruiker toegang heeft tot de pagina, gebruik ik dit script:
(hoe lager het ACL-level, hoe meer toegang je hebt)

PHP:

if ($_SESSION['acl'] > $acl) {
            die('Toegang Geweigerd');
        }

Dit is de database-table waarin de user-informatie staat:

uid	username	password	acl
1	test	098f6bcd4621d373cade4e832627b4f6	0

Bedankt

[ Voor 4% gewijzigd door X_lawl_X op 02-10-2009 20:21 ]

[gvanh]

$password = md5($_POST['password']);

Ik zou hier wel nog een trim() op de $_POST['password'] toevoegen.
Niet wegens veiligheid, maar gebruikers kopieren vaak hun wachtwoord uit een documentje, en dankzij "smart selection" wordt er dan niet zelden een extra spatie mee-geselecteerd. Ik vind het altijd strontvervelend als ik dan vervolgens een foutmelding krijg.

Maar ... nogmaals ... dan heb ik verder niet naar veiligheid gekeken.

[SKiLLa]

Waarom zou je het wachtwoord in een cookie opslaan ? Kun je niet beter een session-ticket gebruiken ?
En ik zou nog zeker 'salt' aan de MD5 hash toevoegen, de toegevoegde waarde van 'standaard MD5' is zo ongeveer nul met alle rainbow tables ...

[donquix]

Hij set alleen de cookies nooit lijkt mij. Je stuurt in regel 55 een location header en pas daarna set je de cookies. Hij komt dus nooit bij het setten van de cookies aan.

Voor de rest lijkt me het opslaan van het wachtwoord in een cookie ook enigszins discutabel maar voor de rest ziet er er netjes uit dacht ik zo.

SKiLLa schreef op vrijdag 02 oktober 2009 @ 20:22:
En ik zou nog zeker 'salt' aan de MD5 hash toevoegen

Ben ik volledig mee eens!

[ Voor 23% gewijzigd door donquix op 02-10-2009 20:25 ]

[mithras]

Hier even wat opmerkingen:
[list]
• Ga eens kijken naar challenge/response systemen tijdens inloggen
• Bewaar slechts de sessie-id in een cookie / sessie en verder de gegevens in de database, geen username/password in dat soort dingen!
• Kijk eens naar SHA1, is wat sterker dan MD5
• Sessie-id per sessie behouden, niet altijd een nieuw id genereren
• Mogelijkheid om ip-lock in te voeren
• Sessie verlooptijd, door gebruiker in te stellen. Nu is sessie altijd geldig. Sowieso by-default een niet-oneindige sessietijd instellen (ook in db!)
Misschien zijn er nog een paar, maar dit is het eerste wat ik direct al opmerk

Daarnaast: jouw acl is geen acl. Het is slechts een hiërarchische level-persmissie integer. Een echte acl heeft wat meer flexibiliteit

[ Voor 9% gewijzigd door mithras op 02-10-2009 20:27 ]

[X_lawl_X]

SKiLLa schreef op vrijdag 02 oktober 2009 @ 20:22:
Waarom zou je het wachtwoord in een cookie opslaan ? Kun je niet beter een session-ticket gebruiken ?
En ik zou nog zeker 'salt' aan de MD5 hash toevoegen, de toegevoegde waarde van 'standaard MD5' is zo ongeveer nul met alle rainbow tables ...

Session-ticket? Ik ben nog maar een PHP-newb eigenlijk en ik weet niet precies wat je daar mee bedoelt . Van Google wordt ik ook niks wijzer

En dat salt snap ik ook niet

edit: Nu wel denk ik. Salten is dus niks anders dan een extra zin/woord aan het password toevoegen?

Hij set alleen de cookies nooit lijkt mij. Je stuurt in regel 55 een location header en pas daarna set je de cookies. Hij komt dus nooit bij het setten van de cookies aan.

Het werkt wel bij mij. Ik blijf ingelogd als ik 'Ingelogd blijven' aanvink en mijn browser opnieuw opstart.

mithras schreef op vrijdag 02 oktober 2009 @ 20:26:
• Sessie-id per sessie behouden, niet altijd een nieuw id genereren

Ik had gelezen dat het veiliger was als je het steeds vernieuwde . Is het dan nutteloos om steeds een nieuwe te genereren?

[ Voor 40% gewijzigd door X_lawl_X op 02-10-2009 20:43 ]

[Tuinstoel]

Ik vind het allemaal wel een beetje dubbelop, en sessies èn cookies? Wat is het nut daarvan? Ook het feit dat je 3 verschillende cookies/sessies gebruikt vind ik beetje raar (array's?). Maar volgensmij is het wel veilig als ik er zo op kijk.

[Orion84]

mithras schreef op vrijdag 02 oktober 2009 @ 20:26:
Hier even wat opmerkingen:
[list]
• Kijk eens naar SHA1, is wat sterker dan MD5

SHA1 vs. MD5 is niet echt heel erg boeiend in deze. De kwetsbaarheden in MD5 hebben enkel betrekking op mogelijkheden om twee strings te maken die dezelfde MD5 hash hebben (strong collision resistance). Een wachtwoord achterhalen als je de MD5 hash hebt (pre-image resistance en weak collision resistance) is nog altijd gewoon een kwestie van bruteforcen.

Let wel: dan laat ik rainbow tables even buiten beschouwing. Wellicht dat er voor SHA1 nog niet zoveel rainbow tables beschikbaar zijn als voor het veelgebruikte MD5. Maar als je je druk maakt over rainbow tables (wat op zich een gerechtvaardigde zorg zou zijn) dan kan je beter gewoon salting toepassen, in plaats van een ander hashalgoritme te gaan gebruiken.

[ Voor 18% gewijzigd door Orion84 op 02-10-2009 20:36 ]

[X_lawl_X]

Tuinstoel schreef op vrijdag 02 oktober 2009 @ 20:28:
Ik vind het allemaal wel een beetje dubbelop, en sessies èn cookies? Wat is het nut daarvan? Ook het feit dat je 3 verschillende cookies/sessies gebruikt vind ik beetje raar (array's?). Maar volgensmij is het wel veilig als ik er zo op kijk.

Ik begon het script met sessies, alleen je moest steeds opnieuw inloggen als je de browser opnieuw opstart. Ik heb toen later de cookies toegevoegd. Ik heb er niet aan gedacht dat de sessies overbodig waren D:

Wat betreft de array's, dit is nog iets te ingewikkeld voor mij . Dit is pas mijn 2e script.

Bedankt voor de hulp tot nu toe @ iedereen

[mithras]

Orion84 schreef op vrijdag 02 oktober 2009 @ 20:31:
SHA1 vs. MD5 is niet echt heel erg boeiend in deze. De kwetsbaarheden in MD5 hebben enkel betrekking op mogelijkheden om twee strings te maken die dezelfde MD5 hash hebben. Een wachtwoord achterhalen als je de MD5 hash hebt is nog altijd gewoon een kwestie van bruteforcen.

Collisions zoeken kostte in 2005 gemiddeld drie kwartier op een Pentium 4 1.6Ghz (bron). Ik zou dus sowieso voor een sterker hash algoritme kiezen (buiten rainbow tables). hoewel SHA1 ook niet zo sterk meer is (bron).

/edit: laat maar, je hebt wel gelijk. In de eerste bron staat dat het zoeken van collisons binnen hashes buiten het probleem ligt.

[ Voor 7% gewijzigd door mithras op 02-10-2009 20:37 ]

[Orion84]

mithras schreef op vrijdag 02 oktober 2009 @ 20:36:
[...]
Collisions zoeken kostte in 2005 gemiddeld drie kwartier op een Pentium 4 1.6Ghz (bron). Ik zou dus sowieso voor een sterker hash algoritme kiezen (buiten rainbow tables). hoewel SHA1 ook niet zo sterk meer is (bron).

Maar aan collisions heb je niks voor het aanvallen van een loginsysteem, collisions zijn interessant bij het gebruik van hashes bij het ondertekenen van certificaten en dergelijke.

Je moet om een loginsysteem aan te vallen die ene collision hebben die hoort bij de hash van iemands wachtwoord. Dat je twee strings kan vinden die dezelfde hashwaarde opleveren wil niet zeggen dat je gegeven een hashwaarde een string kan vinden die exact die gegeven hashwaarde oplevert.

Edit: ah, je was er zelf ook al achter

[mithras]

Orion84 schreef op vrijdag 02 oktober 2009 @ 20:39:
[...]

Maar aan collisions heb je niks voor het aanvallen van een loginsysteem, collisions zijn interessant bij het gebruik van hashes bij het ondertekenen van certificaten en dergelijke.

Je moet om een loginsysteem aan te vallen die ene collision hebben die hoort bij de hash van iemands wachtwoord. Dat je twee strings kan vinden die dezelfde hashwaarde opleveren wil niet zeggen dat je gegeven een hashwaarde een string kan vinden die exact die gegeven hashwaarde oplevert.

offtopic:
Zie mijn edit

Edit: ah, je was er zelf ook al achter

offtopic:
Ja

[ Voor 5% gewijzigd door mithras op 02-10-2009 20:40 ]

[IStealYourGun]

Maakt eigeljk niet veel uit, ik denk dat zowel elke security guru er eens mee is om een sterker algoritme te gebruiken. Voor de rest is het meeste hier wel al gezegd.

Misschien je eens verdiepen in de vele paper i.v.m php security die te vinden zijn op het net?

[X_lawl_X]

mithras schreef op vrijdag 02 oktober 2009 @ 20:26:
Hier even wat opmerkingen:
[list]
• Bewaar slechts de sessie-id in een cookie / sessie en verder de gegevens in de database, geen username/password in dat soort dingen!

Moet ik dit dan zo doen?

bij het loginscript als er is aangevinkt dat er cookies weggezet moeten worden, het sessie id in een cookie stoppen:

PHP:

//indien gewenst, cookies instellen 
            if (isset($_POST['setcookie'])) {
                $ses_id = session_id();
                setcookie("login", "".$ses_id."", time()+60*60*24*31);
                $query = $query = "UPDATE `".$dbnaam."`.`Users` SET `ses_id` = '".$ses_id."' LIMIT 1 ;";
                mysql_query($query) or die('Er is iets mis met de Database');
            }
            header("location: ucp.php");

Bij het controle-script wat op iedere pagina required wordt:

PHP:

elseif (isset($_COOKIE['username'])) { 
        $ses_id = mysql_real_escape_string($_COOKIE['login']); 
                 
        $query = mysql_query("SELECT * FROM Users WHERE ses_id='".$ses_id."'"); 
         
        if (mysql_num_rows($query) == "1") { 
            while($row = mysql_fetch_array($query)) { 

                $_SESSION['uid'] = $row['uid']; 
                $_SESSION['username'] = $row['username']; 
                $_SESSION['password'] = $row['password']; 
                $_SESSION['acl'] = $row['acl']; 
                $_SESSION['ua'] = $_SERVER['HTTP_USER_AGENT']; 
                 
            } 
         
        } else { 
            // cookie laten verlopen
            setcookie("login", "", time()-3600); 
         } 
    }

^is dat dan dus veiliger?

edit: Ik snap nu dus waarom dat het sessie ID niet op iedere pagina veranderd moet worden

[HuHu]

Wachtwoord nooit in de sessie zelf opslaan.

[mithras]

Er zijn al veel topics op GoT die hier over gaan, dus zoek ook even eea op. Maar je zou er zo'n systeem op na kunnen houden:
[list=1]
• Kijk of een cookie bestaat met een sessie-id. Zo ja, pak dat id
• Anders: kijk of een sessie actief is. Pak dan dat id
• Haal de gegevens op uit de database (zijnde: user-id, sessie-id, ip adres, ip-lock switch, ttl)
• Kijk of de ttl verstreken is (kan je ook al in je query doen)
• Kijk, wanneer je ip lock switch aan staat, of de ip adressen matchen
• Haal, wanneer alles juist is, de gegevens van de user op. Sla deze op (bijv. in een registry klasse).

Voor het inloggen krijg je zoiets als:
[list=1]
• Na submit username/password combinatie controleren (nog steeds: challenge/response systemen zijn daar nuttig bij!)
• Genereer nieuw sessie id. Sla dit id samen met de gegevens (user id, ip, ip lock, ttl etc) op in de database (sessie tabel)
• Wanneer een persistent login is aangevraagd (dus langer dan dat een sessie duurt), maak een cookie aan met sessie id en ttl die is aangegeven.
Je hebt nu nog wat quirks. Bijv, in het eerste script een string "maken" door twee keer met een lege string te concatten session_id() retourneert gewoon een string hoor
Daarnaast gaat de query op regel 5 zo ook niet werken

Tot slot: je hebt NOOIT je password gegevens meer nodig wanneer je geauthenticeerd bent. Autorisatie kan puur gebeuren op basis van je user id. Dus ik zou a) het niet in een sessie stoppen en b) niet al deze gegevens. Daarnaast, wat is de reden van de user agent in een sessie opslaan als je er ook via $_SERVER bij kan?

[HuHu]

Opslaan van de user-agent is handig om een eventuele sessie-hijack te detecteren. Als gedurende de sessie je gebruiker van user-agent wisselt, dan is het aannemelijk dat de sessie overgenomen is door een kwaadwillende derde partij. Gewone gebruikers wisselen namelijk niet van user-agent tijdens een sessie.

Maargoed... dan moet je het wel op die manier gebruiken natuurlijk en niet domweg opslaan omdat je dat ergens gelezen hebt, maar eigenlijk niet snapt waarom je dat doet .

[Janoz]

X_lawl_X schreef op zaterdag 03 oktober 2009 @ 09:03:
Moet ik dit dan zo doen?

bij het loginscript als er is aangevinkt dat er cookies weggezet moeten worden, het sessie id in een cookie stoppen:

Sessies in php werken al met een cookie waarin de sessieid opgeslagen word.

Je probeert je probleem (uitgelogd raken bij sluiten browser) op de verkeerde manier op te lossen. Wat je beter kunt doen is even bij sessions in de manual kijken. Daar staat hoe je de eigenschappen van het cookie zo aan kunt passen dat hij langer beschikbaar blijft.

[X_lawl_X]

Bedankt voor de replies allemaal, Ik zie dat ik nog wat te doen heb in het weekend

@Janoz - Zal ik doen, bedankt.

edit:

Ok, ik heb er al wat aan gedaan. Je sessie id wordt nu opgeslagen in de database, indien gewenst wordt er ook een cookie met het sessie id weggezet.

Als je op een pagina komt, wordt er gekeken naar je huidige sessie id of het sessie ID in je cookie, en daar worden je user-gegevens uitgehaald. (access , naam, user id etc).

Ook heb ik er een salt aan toegevoegd, en een IP check optie.

Het volgende script, controller.php, wordt nu op iedere pagina gerequired.

PHP:

<?php
    session_start();
    require('config.php');
    //al ingelogd?
    if (isset($_COOKIE['login'])) {
        $query = mysql_query("SELECT * FROM Users WHERE sessie_id ='".mysql_real_escape_string($_COOKIE['login'])."'");
    } elseif (isset($_SESSION['login'])) {
        $query = mysql_query("SELECT * FROM Users WHERE sessie_id ='".session_id()."'");
    }

    //alleen doen als er ingelogd is
    if (isset($_SESSION['login']) OR isset($_COOKIE['login'])) {
        if (mysql_num_rows($query) != "1") {
            session_destroy();
            setcookie("login", "", time()-3600);
            die('Sessie mismatch - Log opnieuw in');
        }
        
        while ($row = mysql_fetch_array($query)) {
            $db_username = $row['username'];
            $db_uid = $row['uid'];
            $db_session_id = $row['sessie_id'];
            $db_ipcheck = $row['ip_check'];
            $db_ip = $row['user_ip'];
            $db_access = $row['acl'];
            $db_password = $row['password']; //nodig voor wachtwoord veranderen
        }
        
        if ($db_ipcheck == "ja") {
            if ($_SERVER['REMOTE_ADDR'] != $db_ip) {
                session_destroy();
                setcookie("login", "", time()-3600);
            }
        }
    } else {
        //We hebben te maken met een gast
        $db_access = 3;
    }
?>

En hier is het login script

PHP:

<?php
    require('config.php');
    require('controller.php');

    //controleren of dat er al ingelogd is
    if ($db_access != "3" && $_GET['q'] != "logout") {
        
        //terug naar de homepage
        header ("location: ucp.php");

        exit;

    }

    //Inloggen
    if ($_GET['ver'] == "true") {

        $username = mysql_real_escape_string($_POST['username']);
        $password = sha1($salt.$_POST['password']);

        //inlog controleren
        $query = mysql_query("SELECT * FROM Users WHERE username='".$username."' AND password='".$password."'");

        if (mysql_num_rows($query) == "1") {
            session_regenerate_id();
            $_SESSION['login'] = "ja";
        
            $query = "UPDATE `".$dbnaam."`.`Users` SET `sessie_id` = '".session_id()."', `user_ip` = '".$_SERVER['REMOTE_ADDR']."' WHERE username='".$username."' AND password='".$password."' LIMIT 1 ;";
            mysql_query($query) or die('Er is iets mis met de Database');
        
            
            
            //indien gewenst, cookies instellen
            if (isset($_POST['setcookie']) && $allowcookies == "ja") {
                setcookie("login", "".session_id()."", time()+60*60*24*31);
            }
            
            header("location: ucp.php");

        } else {

            //wachtwoord of username fout?

            $msg = "<p>Het wachtwoord of de gebruikersnaam is ongeldig.</p>";

        }
    }

    

    //uitloggen

    if ($_GET['q'] == "logout") {

        session_destroy();
        
        if ($allowcookies == "ja") {
            // cookies laten verlopen als ze bestaan
            setcookie("login", "", time()-3600);
        }
        
        $msg = "<p>U bent uitgelogd.</p>";

    }

?>


<?php echo $msg; ?>

<form action="ucp.php?q=login&ver=true" method="post">
<p>Gebruikersnaam:<br />
<input type="text" name="username"></p>
<p>Wachtwoord:<br />
<input name="password" type="password" value="" /></p>
<?php if ($allowcookies == "ja") { ?><p><input type="checkbox" name="setcookie" /> Ingelogd blijven?</p><?php } ?>
<input type="submit" class="alinea" value="Log In">
</form>

Ik ben er nog niet helemaal mee klaar, ik ga morgen ook nog de rest van de tips van mithras toevoegen. Ook moet ik de User agent-controle er opnieuw inzetten

Ik geloof dat dit script al beter is dan het vorige aangezien er geen password met een sessie wordt meegestuurd.

note: Ik heb het nog niet getest. Kan best zijn dat er nog fouten inzitten

[ Voor 94% gewijzigd door X_lawl_X op 04-10-2009 10:53 ]

[X_lawl_X]

Er is nu iets geks aan de hand D:

Als je niet ingelogd bent, kun je toch op iedere beveiligde pagina van de site komen. Dit gebeurd alleen als ik de volgende functie require, niet als ik het script direct op de beveiligde pagina plaats en $acl vervang door een nummer.

[code=php]function access($acl) {
if ($db_access > $acl) {
header("location: fout.php");
exit;
}
}[/code]

Deze functie werkte perfect totdat ik $_SESSION['acl'] verving door $db_access. Weet iemand waarom?

edit: zelf al opgelost. Ik was vergeten om $db_access ook als argument weg te zetten in de functies

anyways, is het nu al veiliger?

[ Voor 13% gewijzigd door X_lawl_X op 04-10-2009 10:52 ]

[Voutloos]

Een aantal stijl puntjes:
• Het meeste is Engels, maar dan noem je een kolom opeens sessie_id.
• Omdat sessie_id een attribuut van User is, kan je maar op 1 pc tegelijk ingelogd zijn, wat niet gebruiksvriendelijk is.
• mysql_num_rows($query) != "1" . Die 1 hoeft niet binnen quotes.
• while($row=...) is wat overdreven als je al weet dat het 1 row is
• Er staan magische getallen in, bijv. 3
• 'ver' parameter is overbodig, of stuur hem door via POST
• Ik zie een redirect header zonder direct volgende die()


En de belangrijkste:
Deze code is wel heel ongestructureerd zonder functies, wat OO en de noodzakelijke zooi als user en db classes. Ik zelfs durven stellen dat als je met deze stijl door gaat met het uitbrieden van je scripts dat je juist veiligheidsproblemen creert cq over het hoofd ziet.

[X_lawl_X]

Voutloos schreef op zondag 04 oktober 2009 @ 11:33:
Een aantal stijl puntjes:
• Het meeste is Engels, maar dan noem je een kolom opeens sessie_id.
• Omdat sessie_id een attribuut van User is, kan je maar op 1 pc tegelijk ingelogd zijn, wat niet gebruiksvriendelijk is.
• mysql_num_rows($query) != "1" . Die 1 hoeft niet binnen quotes.
• while($row=...) is wat overdreven als je al weet dat het 1 row is
• Er staan magische getallen in, bijv. 3
• 'ver' parameter is overbodig, of stuur hem door via POST
• Ik zie een redirect header zonder direct volgende die()


En de belangrijkste:
Deze code is wel heel ongestructureerd zonder functies, wat OO en de noodzakelijke zooi als user en db classes. Ik zelfs durven stellen dat als je met deze stijl door gaat met het uitbrieden van je scripts dat je juist veiligheidsproblemen creert cq over het hoofd ziet.

Ik gebruik nog geen classes omdat ik daar nog niks van snap XD. Wat betreft de while($row), is er dan een andere manier om de gegevens uit de database te halen? 0.0

[Jannis]

Bij een while-lus wordt de tussenliggende code voor elke row uitgevoerd, maar omdat je hier maar 1 row hebt is dat niet nodig, en kun je de while( ) gewoon weglaten.

Het wordt dan dus:

PHP:

$row = mysql_fetch_array($query);

[X_lawl_X]

Dat ik daar niet zelf opgekomen ben. Bedankt!

[EdwinG]

Even terug op je eerste stuk code (gelukkig al veranderd):

Inlogpagina, regel 46:

PHP:

 $_SESSION['username'] = $row['username'];

Controlescript, regel 19:

PHP:

$query = mysql_query("SELECT * FROM Users WHERE username='".$_SESSION['username']."' AND password='".$_SESSION['password']."' AND uid='".$_SESSION['uid']."' AND acl='".$_SESSION['acl']."'");

Als iemand zich registreerd met de volgende gebruikersnaam:

code:

'; TRUNCATE TABLE users; --

Komt, uitgaande van goede afhandeling tijdens de registratie, de gebruikersnaam letterlijk in de database. Je mag in dat geval blij zijn dat mysql_query() geen stacked queries ondersteund, want je zou erg weinig gebruikers overhouden.

[Cartman!]

PHP:

$_SESSION['login'] = "ja";

Opzich niet erg maar pak gewoon booleans, daar zijn ze voor

[sky-]

Wat heeft het voor nut om session['login'] op true (of 'ja') te zetten? Want, je kan beter een user id met een of andere hash hebben, dan controleer je naar de db of deze aanwezig zijn en dan heb je een sessie.

[CodeCaster]

X_lawl_X schreef op zondag 04 oktober 2009 @ 12:27:
Ik gebruik nog geen classes omdat ik daar nog niks van snap XD.

Classes zijn geen dingen om te gebruiken, het is niet dat je, wanneer je class {} om een blok code zet, ineens "classes gebruikt".

Je hebt in dit topic 331 regels code geplaatst, met daarbij telkens de vraag of je goed bezig bent. Die vraag beantwoordt zichzelf eigenlijk al. Door het posten van code laat je zien dat je bezig bent met de implementatie, en wanneer je daar mee bezig bent, ben je de "waarom"- en "hoe"-vraag al voorbij. Met deze vragen kun je jezelf ondervragen wat je gaat doen. "Waarom ga ik dit doen" (probleemstelling) en "hoe ga ik dit doen" (ontwerp, planning) moet je jezelf vragen vóór er ook maar één letter code in je editor staat.

Weet je wáárom je dit wil programmeren, en hoe je het probleem dat je hebt wil gaan oplossen?

[mcdronkz]

Even iets tussendoor, waarom is de standaard configuratie dusdanig dat de cookies die het sessie-id bevatten meteen verwijderd worden wanneer de browser opnieuw wordt opgestart?

[CodeCaster]

mcdronkz schreef op maandag 05 oktober 2009 @ 01:51:
Even iets tussendoor, waarom is de standaard configuratie dusdanig dat de cookies die het sessie-id bevatten meteen verwijderd worden wanneer de browser opnieuw wordt opgestart?

In tegenstelling tot wat ik in mijn vorige post beweerde is "waarom" geen vraag die je al te veel moet stellen wanneer je je met de werking van PHP bezig houdt.

[mcdronkz]

CodeCaster schreef op maandag 05 oktober 2009 @ 02:00:
[...]

In tegenstelling tot wat ik in mijn vorige post beweerde is "waarom" geen vraag die je al te veel moet stellen wanneer je je met de werking van PHP bezig houdt.

Ik denk dat je daar wel een punt hebt . Echter, ik ben toch benieuwd. Als ik de sessie een jaar lang zou bewaren, brengt dat dan risico's met zich mee? Is dat de reden van de standaardwaarde?

Dan nog iets, er is een session cache expire. Is de session cache de map waarin de bestanden met de belangrijke waarden (gebruikersnaam, wachtwoord, noem maar op..) staan? Deze waarde staat standaard op 180 minuten. Als ik het dus goed begrijp is een sessie standaard na 3 uur verdwenen, ongeacht de cookie lifetime. Wanneer je de cookie lifetime op een jaar zet zul je dus ook de session cache expire op minstens een jaar moeten zetten. Of zit ik nu verkeerd?

Misschien wat basic vragen, maar wellicht is de topicstarter hier ook mee geholpen.

[CodeCaster]

quote: http://php.net/session-cache-expire

The manual probably doesn't stress this enough:

** This has nothing to do with lifetime of a session **

Whatever you set this setting to, it won't change how long sessions live on your server.

This only changes HTTP cache expiration time (Expires: and Cache-Control: max-age headers), which advise browser for how long it can keep pages cached in user's cache without having to reload them from the server.

[mcdronkz]

Oké, stom van me. Dat had ik kunnen weten. Dan blijf ik met één vraag zitten; wat bepaald wél hoe lang een sessie op een server blijft bestaan?

[CodeCaster]

quote: http://www.captain.at/howto-php-sessions.php

PHP has a sort of built-in "load-balancing" feature for this garbage collector, so that old session files are not deleted on each and every session request, but with a certain probability. The default timeout for session files is 1440 seconds or 24 minutes.

En nu slapen

[Cartman!]

Om zulke ellende te voorkomen van ineens uitgelogd zijn en niet weten waarom is een eigen sessionhandler altijd zo ideaal omdat je alles zelf in de hand kunt houden.