Door
Kevin de Groot
op 15-12-2010 16:47
gewijzigd op 15-12-2010 17:14
3.796 views
Beste mensen,
Momenteel ben ik bezig om een veilig loginsysteem te bouwen voor een bekende van me. Nu luidt de vraag; is mijn beveiligingssysteem (redelijk) veilig?
- De gebruiker logt in met de gegevens op de loginpagina. Indien alles correct is zal het systeem de sessies aanmaken voor de gebruiker. Één sessie voor het gebruikersid, één voor het ip-adres en één voor de sleutel.
- Op een 'beveiligde pagina' wordt de gebruiker gechecked door middel van de functie die hieronder staat.
Note 1: de inlogpagina is volgens mij wel veilig, maar kan ik op verzoek ook wel even plaatsen.
Note 2: de functie werkt gewoon dus ik test hem wél.
Kortom; is deze functie veilig genoeg?
<?php
function getLevel()
{
if (isset($_SESSION['gebruikersid']) AND !empty($_SESSION['gebruikersid']) AND ctype_digit($_SESSION['gebruikersid']))
{
if (isset($_SESSION['ipadres']) AND !empty($_SESSION['ipadres']) AND $_SESSION['ipadres'] == $_SERVER['REMOTE_ADDR'])
{
if (isset($_SESSION['sleutel']) AND !empty($_SESSION['sleutel']) AND strlen($_SESSION['sleutel']) == 40)
{
$qSelect = mysql_query("SELECT gebruiker_actief
FROM gebruikrs
WHERE gebruiker_id = '". veiligeInvoer($_SESSION['gebruikersid']) ."'
AND inlog_ipadres = '". veiligeInvoer($_SESSION['ipadres']) ."'
AND inlog_sleutel = '". veiligeInvoer($_SESSION['sleutel']) ."'
");
if (mysql_num_rows($qSelect) == 1)
{
$qFetch = mysql_fetch_assoc($qSelect);
htmlentities is een functie die je beschermt tegen (al dan niet kwalijke) html.
Kwestie dat mensen geen actieve html kunnen posten (post bv. dit: "</div></div></div>" en de kans is vrij groot dat je hele lay out naar de knoppen is).
htmlentities gebruik je dan ook bij het tonen van gegevens naar de client. De functie dient niet om je database te beschermen.
Ik zie dus niet goed wat dat daar komt te staan.
Ik zal zelf ook eens testen.
Nog iets: als je een integer verwacht, cast die dan naar int. Meer bescherming heb je niet nodig.
bv. WHERE gebruiker_id = '". (int) $_SESSION['gebruikersid'] ."'
----------------------------------
PS.
@ PHP Scripter:
else if ( $sessie = false ) {
Dit slaat nergens op.
Ten eerste hoort daar een dubbele =
Ten tweede, ik zie nergens $sessie = true; staan.
htmlentities is een functie die je beschermt tegen (al dan niet kwalijke) html.
Kwestie dat mensen geen actieve html kunnen posten (post bv. dit: "</div></div></div>" en de kans is vrij groot dat je hele lay out naar de knoppen is).
htmlentities gebruik je dan ook bij het tonen van gegevens naar de client. De functie dient niet om je database te beschermen.
Ik zie dus niet goed wat dat daar komt te staan.
Ik zal zelf ook eens testen.
----------------------------------
PS.
@ PHP Scripter:
else if ( $sessie = false ) {
Dit slaat nergens op.
Ten eerste hoort daar een dubbele =
Ten tweede, ik zie nergens $sessie = true; staan.
Dat heb ik gedaan voor overige invoer (voor bijvoorbeeld berichten) en dacht dat het geen kwaad zou doen om deze functie ook hier te gebruiken. Vandaar die htmlentities.
(gepost vóór de laaste aanpassing vorige post)
Dan nog ...
Het is beter om data van gebruikers zo puur mogelijk op te slaan. Juist beschermen tegen injection.
Je moet dan wel telkens bij het tonen van de data elk veld controleren.
Een argument: stel dat een gebruiker zijn post wil aanpassen, dan zal hij niet snappen dat daar van alles staat dat hij niet zelf heeft gepost.
Oké, ik kreeg het namelijk niet voor elkaar zonder die htmlentities. Maar beveiliging voor de database (injecties) kan dus gewoon beter met addslashes of mysql_real_escape_string, toch?
Oké, ik heb wijzigingen doorgevoerd en het ziet er nu zo uit:
Note: ik heb de else-jes laten staan. Dit vind ik sowieso wat overzichtelijker.
<?php
function mijnGebruikerslevel()
{
if (!empty($_SESSION['gebruikersid']) AND ctype_digit($_SESSION['gebruikersid']))
{
if (!empty($_SESSION['ipadres']) AND $_SESSION['ipadres'] == $_SERVER['REMOTE_ADDR'])
{
if (!empty($_SESSION['sleutel']) AND strlen($_SESSION['sleutel']) == 40)
{
$gSelect = mysql_query("SELECT COUNT(gebruikersid) AS aantal_resultaten
FROM gebruikers
WHERE gebruiker_id = '". (int)$_SESSION['gebruikersid'] ."'
AND gebruiker_actief = '1'
AND login_ipadres = '". $_SESSION['ipadres'] ."'
AND login_sleutel = '". $_SESSION['sleutel'] ."'
ORDER BY gebruiker_id
LIMIT 0,1
");
$gFetch = mysql_fetch_assoc($gSelect);
