loginscript (veilig?)

Wouter OOP

07/05/2009 19:09:00

Hallo allemaal, ik ben nieuw met php en mysql en ben een maand of 2 geleden begonnen met het leren ervan.
Nou heb ik een tutorial gevolgd om een loginscript te maken, nou vroeg ik me af of het misschien nog beter beveiligd moet worden.

Het zijn de volgende files:

login_check.php:

Code (php)

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32

<?php

$inlognaam = $_POST['inlognaam'];
$wachtwoord = $_POST['wachtwoord'];

if ($inlognaam&&$wachtwoord)
{
$connect = mysql_connect ("localhost","root","") or die ("Kan niet verbinden");
mysql_select_db ("test") or die ("Kan db niet vinden");

$inlognaam  = stripslashes($inlognaam);
$wachtwoord = stripslashes($wachtwoord);
$inlognaam  = mysql_real_escape_string($inlognaam);
$wachtwoord = mysql_real_escape_string($wachtwoord);

$query = mysql_query ("SELECT * FROM leden WHERE inlognaam='$inlognaam' AND wachtwoord='$wachtwoord'");

$numrows = mysql_num_rows ($query);

if ($numrows !=0)
{
session_register("inlognaam");
session_register("wachtwoord");
header("location:login_success.php");
}
else
    die ("Inlognaam / Wachtwoord klopt niet!");
}
else
    echo "Vul inlognaam en wachtwoord in!";

?>

login_succes.php:

Code (php)

1
2
3
4
5
6
7
8
9

<?php

session_start();

if(!session_is_registered("inlognaam")){
header("location:login_form.php");
}

?>

<html>

<body>
Ingelogd!<br/>
<a href=logout.php>Log out!</a>
</body>

</html>

Bedankt,

Wouter

PHP hulp

25/05/2012 08:26:54

Gesponsorde koppelingen:

Mark PHP

07/05/2009 19:21:00

Een aantal punten die me direct opvallen:
- wachtwoord ongecodeerd in database, gebruik hash(),
- onnodige stripslashes,
- foutafhandeling ontbreekt,
- session_register en session_is_registered zijn verouderd, gebruik $_SESSION,
- de laatste regels van login_check.php kloppen niet: dubbele else.

Gewijzigd op 01/01/1970 01:00:00 door Mark PHP

Emmanuel Delay

07/05/2009 19:21:00

Nee, dit is een slecht idee. (EDIT: ik spreek uiteraard niet over de post boven mij.)

Eerst en vooral: je slaat je wachtwoorden zomaar op, niet gecodeerd. Leuk. Zo kan je de paswoorden van iedereen zomaar in de db zien.

paswoorden zet je ook nooit in een sessie.
Het paswoord heb je enkel nodig op het moment van het inloggen. Daarna laat je dat paswoord gerust.

Gewijzigd op 01/01/1970 01:00:00 door Emmanuel Delay

Afra ca

07/05/2009 19:40:00

Agirre heeft gedeeltelijk gelijk over de dubbele else.

Als je goed zou inspringen zul je zien dat de eerste else nog van een if statement in de if statement komt. Desondanks klopt hij niet omdat je bij de 1-na-laatste else de brackets ( "{" en "}" ) vergeet!

edit:

* onnodig variabelen gaan definieren, je kan prima werken met post variabelen, en de stripslashes kun je tegelijk in de query toepassen.

* variabelen buiten quotes halen (bijv. in query)

* Mocht hash() je als beginner wat lastig lijken, kan je ook prima werken met md5($_POST['wachtwoord']) , alleen wees je ervan bewust dat er tegenwoordig databases worden bijgehouden met bekende md5 wachtwoorden, dus overweeg sha1 (iets minder bekend, maar ook daar als DB's voor), of dus gewoon wel de hash() functie

edit 2:

* Je moet zelf even wat onderzoek doen naar de goede combinatie van stripslashes & mysql_real_escape_string (ivm. magic quotes die soms aan en soms uit staan)

Gewijzigd op 01/01/1970 01:00:00 door Afra ca

Mr.Ark

07/05/2009 20:02:00

Even snel je code herschreven, Zoals eerder gezegd, Sla je wachtwoord als MD5 op. Zorg ook dat je sql injection tegen gaat, En dat je goeie checks uitvoert.

Code (php)

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61

<?PHP
/* Inlognaam check */
if(isset($_POST['inlognaam'] && !empty($_POST['inlognaam']))
{
    /* Wachtwoord check */
    if(isset($_POST['wachtwoord'] && !empty($_POST['wachtwoord']))
    {
        /* Query uitvoeren */
        $Query =
        "
            SELECT
                ID
            FROM
                tabel
            WHERE
                inlognaam = '".mysql_real_escape_string($_POST['inlognaam'])."'
            AND
                wachtwoord = '".md5($_POST['wachtwoord'])."'
        ";

        /* Result */
        $Result = mysql_query($Query);

        /* Kijken of query is gelukt. */
        if(!$Result)
        {
            echo 'Fout opgetreden met query, '.$Query.'';
        }
        else
        {
            /* Kijken of er een result is. */
            if(mysql_num_rows($Result) !== 0)
            {
                /* Sessies aanmaken. */
                $Rij = mysql_fetch_assoc($Result);

                $_SESSION['USER_ID'] = $Rij['ID'];
                $_SESSION['USER_IP'] = $_SERVER['REMOTE_ADDR'];

                /* Doorsturen */
                header('location: jouwpagina.php');
            }
            else
            {
                /* Geen result. */
                echo 'Geen resultaat gevonden in database';
            }
        }
    }
    else
    {
        /* Geen wachtwoord ingevuld. */
        echo 'Geen wachtwoord ingevuld.';
    }
}
else
{
    /* Geen inlognaam ingevuld. */
    echo 'Geen inlognaam ingevuld.';
}
?>

Gewijzigd op 01/01/1970 01:00:00 door Mr.Ark

Mark PHP

07/05/2009 21:18:00

@Ark
Ziet er al beter uit, alleen waarom zou je het IP vanuit de database selecteren? Dat wil je niet. Als je het IP in een sessie wilt hebben (wat soms handig is), gebruik dan $_SERVER['REMOTE_ADDR'].

@Afra
Je kan md5 ook prima met hash() voor elkaar krijgen.

Gewijzigd op 01/01/1970 01:00:00 door Mark PHP

Mr.Ark

07/05/2009 21:22:00

@ Agirre

Je hebt gelijk, uit snelheid had ik daar niet bij nagedacht.
Zal het even verbeteren.

Afra ca

07/05/2009 21:48:00

@ Agirre,

zeker, aangezien je bij hash als eerste parameter gewoon het algoritme kan aangeven. Maar denk dat

Code (php)

<?php hash(md5,$tekst); ?>

iets lastiger is dan

Code (php)

<?php md5($tekst); ?>

voor een beginner.

Gewijzigd op 01/01/1970 01:00:00 door Afra ca

Mark PHP

07/05/2009 21:50:00

Klopt, voor een beginner is dat inderdaad makkelijker. Alleen hash() is meer flexibel en biedt meer mogelijkheden, dus vandaar dat ik deze altijd gebruik (meestal sha256).

Gewijzigd op 01/01/1970 01:00:00 door Mark PHP

Afra ca

07/05/2009 22:05:00

Stem ik zeker mee in, en ik raad dan ook eigenlijk het gebruik van hash() aan. En met 35 algoritmes is de keuze reuze :)

Begrijp wel, de encryptie (ok, hash is officieel gezien geen encryptie) kan nog zo goed zijn, het gaat om de manier waarop het gebruikt wordt in een systeem. (evt. as easy as SQL INJECTION)

Wouter OOP

07/05/2009 23:37:00

Bedankt voor alle reacties.
Ik vond het al heel wat dat ik begreep wat ik heb geschreven :), kennelijk komt er nog heel wat meer bij kijken.

@ark

Ook jij bedankt voor het herschrijven van het script, ik ga dit zeker gebruiken om te bestuderen.

Nogmaals bedankt,

Wouter

Wouter OOP

08/05/2009 15:05:00

Nou vroeg ik me nog af, waarom isset?

Dat heb ik nog nooit gebruikt, is dit veiliger?

Mr.Ark

08/05/2009 15:35:00

-> http://www.php.net/isset

Quote:

Determine whether a variable is set.

oscar thys

30/04/2012 14:35:15

Ben ik nu mis of heb ik het verkeerd voor,maar volgens mij mankeerd er een ")"afsluit haakje op de eerste If(isset($_POST['inlognaam'] AND !empty($_POST['inlognaam'])))
Dus ik kom op 3 uit
en in het verbeterde script staan er maar 2

Wouter J

30/04/2012 14:44:23

Ben ik nu mis of is dit topic uit 8 mei 2009?

PHPhulp is een Nederlandstalige PHP community sinds 2002. Je vindt hier PHP tutorials, PHP scripts, PHP boeken en nog veel meer. Alles is geheel gratis!