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);

if ($qFetch['gebruiker_actief'] == 1)
{
return 1;
}
else
{
verwijderSessies();
return 0;
}
}
else
{
verwijderSessies();
return 0;
}
}
else
{
verwijderSessies();
return 0;
}
}
else
{
verwijderSessies();
return 0;
}
}
else
{
verwijderSessies();
return 0;
}
}

?>

Alvast bedankt.


Met vriendelijke groet,

Kevin de Groot
Dit moet toch ook werken?


<?php
function getLevel()
{
    if (isset($_SESSION['gebruikersid']) AND !empty($_SESSION['gebruikersid']) AND ctype_digit($_SESSION['gebruikersid']))
    {
        $sessie = false;
        
        if (isset($_SESSION['ipadres']) AND !empty($_SESSION['ipadres']) AND $_SESSION['ipadres'] == $_SERVER['REMOTE_ADDR'])
        {
            $sessie = false;
            
            if (isset($_SESSION['sleutel']) AND !empty($_SESSION['sleutel']) AND strlen($_SESSION['sleutel']) == 40)
            {
                $sessie = false;
                
                $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);
                    
                    if ($qFetch['gebruiker_actief'] == 1)
                    {
                        return 1;
                    }
                    else if ( $sessie = false )
                    {
                        verwijderSessie();
                    }
                }
            }
        }
    }
}
Volgens mij wel gewoon, toch? Is wel een stukje korter, maar goed, zal de functie veilig (genoeg) zijn?
Wat zijn de functies veiligeInvoer en verwijderSessie()?
is dat niet session_destroy();
heb je hem al getest? zoja ? en werkt hij ook
Kevin de Groot op 15/12/2010 16:47:15

<?php

if (isset($_SESSION['gebruikersid']) AND !empty($_SESSION['gebruikersid']) AND ctype_digit($_SESSION['gebruikersid']))
?>

Waarom die !empty controle; je controleert toch met isset al of session bestaat en met ctype controleer je of het een getal is?
Kevin de Groot op 15/12/2010 16:47:15

<?php
if (isset($_SESSION['ipadres']) AND !empty($_SESSION['ipadres']) AND $_SESSION['ipadres'] == $_SERVER['REMOTE_ADDR'])
?>

Hiervoor hetzelfde; waarom die !empty controle?

Kevin de Groot op 15/12/2010 16:47:15

<?php
if (isset($_SESSION['sleutel']) AND !empty($_SESSION['sleutel']) AND strlen($_SESSION['sleutel']) == 40)

?>

Hiervoor hetzelfde; waarom die !empty controle?
En waarom in zoveel regels? Afgaande op de inhoud van de diverse else, kan het volgens mij gewoon in 1x.
Zorg voor foutafhandeling van je query.

Waar komt veiligeInvoer en verwijderSessie vandaan en wat doet die?

Daarnaast: er is verschil tussen AND en &&.
Ik controleer eerst of de sessie geset is, vervolgens of hij niet leeg is (want het is mogelijk dat een sessie geen waarde heeft) en daarna of het een legitieme karakter is (In dit geval 0-9).

De functie 'veiligeInvoer' ziet er als volgt uit:

<?php

function veiligeInvoer($sInput)
{
$sOutput = (function_exists('mysql_real_escape_string')) ? mysql_real_escape_string($sInput) : addslashes($sInput);

return htmlentities($sOutput, ENT_QUOTES);
}

?>

De functie 'verwijderSessies' ziet er als volgt uit:

<?php

function verwijderSessies()
{
if (isset($_SESSION['gebruikersid']))
{
$_SESSION = array();
session_destroy();
}
}

?>
zonder session_start(); werkt session_destroy(); ook niet
Jordi kroon op 15/12/2010 17:09:02

zonder session_start(); werkt session_destroy(); ook niet


Het bestand waar deze functie in staat wordt d.m.v. een include 'toegevoegd' aan de index.php. In de index.php staat session_start();.
Obelix en Idefix op 15/12/2010 17:00:01
... Waarom die !empty controle; ...



Kevin de Groot op 15/12/2010 17:04:33

Ik controleer eerst of de sessie geset is, vervolgens of hij niet leeg is (want het is mogelijk dat een sessie geen waarde heeft)


De functie empty doet zelf al een isset check. Je mag dus empty() gebruiken zonder dat je eerst moet checken op isset().
Kris Peeters op 15/12/2010 17:14:36

[quote="Obelix en Idefix op 15/12/2010 17:00:01"]... Waarom die !empty controle; ...



Kevin de Groot op 15/12/2010 17:04:33

Ik controleer eerst of de sessie geset is, vervolgens of hij niet leeg is (want het is mogelijk dat een sessie geen waarde heeft)


De functie empty doet zelf al een isset check. Je mag dus empty() gebruiken zonder dat je eerst moet checken op isset().
[/quote]

Dit is nieuw voor mij, ik las zoiets dus net ook al op PHP.net. Ik zal het er meteen in verwerken. Dankje. Maar kunnen mensen (lees: hackers) ook eenvoudig inbreken?

Reageren