Gevaarlijk script

Overzicht Reageren

Sponsored by: Vacatures door Monsterboard

J C

J C

28/05/2016 15:13:33
Quote Anchor link
Ik kreeg vandaag een email van een bedrijf die aangaf dat mijn inlog script op mijn website de mogelijkheid heeft om gehacked te worden.

Zij konden dit tegen betaling voor ons oplossen. Op zich helemaal geen probleem.
Maar vaag me nu af of het werkelijk onveilig is.

Zij gaven aan dat ze bij het inloggen de volgende melding kregen. UIteraard willen ze niet aangeven hoe deze foutmelding is ontstaan.

foto van de foutmelding

Wat denken jullie is dit gevaarlijk?
Gewijzigd op 28/05/2016 15:28:30 door J C
 
PHP hulp

PHP hulp

30/12/2024 19:24:13
 
Ward van der Put
Moderator

Ward van der Put

28/05/2016 15:46:04
Quote Anchor link
Post eens de relevante code, te beginnen met de in de stack trace genoemde bestanden rond de genoemde regels
 
J C

J C

28/05/2016 15:59:58
Quote Anchor link
Ben bang dat dat het hele probleem wordt, ik weet niet welke bestanden.

Index.php is namelijk alleen een verzameling van includes en variabelen.

Het enige dat ik kan vinden zou dit kunnen zijn:

Code (php)
PHP script in nieuw venster Selecteer het PHP script
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
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
<?php
if(isset($_GET['logout'])){
    del_inlog();
    header("location: ./index.php");
    exit;    
}


if(isset($_POST['login']))
{

$password = $_POST['mw_pass'];
//check personal salt
$saltqry = $connection->query("
                                SELECT
                                        salt,
                                        mw_gegevens_groep                                        
                                FROM
                                        mw_gegevens
                                WHERE     
                                        mw_gegevens_persnr='"
.$_POST['mw_user']."'
                                "
);

    (
$erroruitkomst = $saltqry->fetch_assoc());
    if($saltqry->num_rows == 0)
    {

        $salt='';
    }

    else
    {
        $salt = $erroruitkomst['salt'];
    }

    
    include_once('inlog/passcrypt.php');

    $userpassword = $Nieuw_ww;

    $mw_gegevens_qry = $connection->query("
                                SELECT
                                        *
                                FROM
                                        mw_gegevens
                                WHERE     
                                        mw_gegevens_persnr='"
.$_POST['mw_user']."'
                                AND     
                                        mw_gegevens_pass='"
.$userpassword."'
                                AND
                                        mw_gegevens_pass!=''
                                "
);

    (
$mwgegevens = $mw_gegevens_qry->fetch_assoc());
    if($_POST['mw_user']=='')
    {

        $aErrors=71;
    }

    elseif($_POST['mw_pass']=='')
    {

        $aErrors=8;
    }

    elseif($_POST['mw_pass']!='' && $_POST['mw_user']!='' && $mw_gegevens_qry->num_rows == 0)
    {

        $aErrors= 91;
    }

    elseif($saltqry->num_rows != 0 && $erroruitkomst['mw_gegevens_groep']==7)
    {

        $aErrors= 11;
    }

    elseif($saltqry->num_rows == 1)
    {

        $aErrors=0;
    }

    else
    {
        $aErrors=9999;
    }

    if($_POST['mw_user']=='')
    {

        $user=0;
    }

    else
    {
    $user=$_POST['mw_user'];
    }

    if($aErrors!=0)
    {

$error=$aErrors;

//inlogerror log
  include($pathmw.'includes/error.php');
     $qry = "
        insert into
            mw_errorlog
        SET
            logintime        =    '"
.mysql_real_escape_string(time())."',
            ipadres            =    '"
.$_SERVER['REMOTE_ADDR']."',
            mwnr            =    ?,
            melding            =    '"
.$aErrors."'
            "
;
        
    $sql = $connection->prepare($qry);
        if($qry === false)
        {

        echo "Query error:.". $connection->error();
        }

        else
        {     $sql->bind_param('i', $user);  
            $sql->execute();
            $sql->close();
    $welkom .= '<br/><font color=red>Fout bij inloggen: ' . $errormessage .' </font><br/><br/>';
        }
    }

    else
    {     
            set_inlog($mwgegevens);
            if(isset($_GET['pagina']))
            {

            header("location: ?pagina=".$_GET['pagina']);
            }

            else
            {
            header("location: index.php?pagina=home");
            }

            exit;
    }
}

?>
Gewijzigd op 28/05/2016 16:04:07 door J C
 
- Ariën  -
Beheerder

- Ariën -

28/05/2016 16:28:39
Quote Anchor link
Op lijn 19/41 is je $_POST onbeveiligd. SQL-injectie is hier mogelijk.
Gewijzigd op 28/05/2016 16:30:34 door - Ariën -
 
J C

J C

28/05/2016 16:32:30
Quote Anchor link
IK dacht dat dat niet meer mogelijk was met deze nieuwe manier van scripten.

Is dit gewoon op te lossen met mysql_real_escape_string?
 
- Ariën  -
Beheerder

- Ariën -

28/05/2016 16:35:45
Quote Anchor link
Ja, maar ik zie dat je objecten ($connection->...) gebruikt voor je database-functies. Gebruik je soms PDO of MySQLi?

Want mysql_real_escape_string evenals dé andere soortgelijke mysql-functies zijn 'deprecated'
 
J C

J C

28/05/2016 16:39:17
Quote Anchor link
Ik gebruik mysqli, daarom dacht ik dat sql injectie niet meer mogelijk was.
 
- Ariën  -
Beheerder

- Ariën -

28/05/2016 16:42:05
Quote Anchor link
Nonsens.....
Daarvoor hebben we dan:
mysqli_real_escape_string of in jouw geval $connection->real_escape_string().
Gewijzigd op 28/05/2016 16:43:20 door - Ariën -
 
Thomas van den Heuvel

Thomas van den Heuvel

28/05/2016 16:49:56
Quote Anchor link
J C op 28/05/2016 15:13:33:
Ik kreeg vandaag een email van een bedrijf die aangaf dat mijn inlog script op mijn website de mogelijkheid heeft om gehacked te worden.

Zij konden dit tegen betaling voor ons oplossen. Op zich helemaal geen probleem.
Maar vaag me nu af of het werkelijk onveilig is.

Zij gaven aan dat ze bij het inloggen de volgende melding kregen. UIteraard willen ze niet aangeven hoe deze foutmelding is ontstaan.

foto van de foutmelding

Wat denken jullie is dit gevaarlijk?

Dit balanceert op de rand van afpersing. Wat vooral gevaarlijk is is ingaan op dit soort aanbiedingen, vooral als je de partij niet kent. Ga je wildvreemden toegang geven tot je broncode, waarom neem je dan uberhaupt nog de moeite om veilige applicaties te bouwen? Geef de dieven meteen de huissleutel :p.

Dat gezegd hebbende, het melden+weergeven van fouten op een productie-omgeving is niet zo strak he, deze zouden enkel in een errorlog moeten komen en niet op het scherm van eindgebruikers.

Daarnaast heb je wat rare aannames over MySQLi. Wanneer je aan de slag gaat met loginsystemen zul je meer van de hoed en de rand moeten weten.
 
J C

J C

28/05/2016 16:54:11
Quote Anchor link
Er is natuurlijk wel een verschil in iemand een script laten schrijven en iemand toegang geven tot een ftpserver.

In principe hebik volgens mij ook alle foutmeldingen opgevangen, ik weet alleen niet hoe deze is geproduceerd.
Elke foutmelding wordt bij mij ook nog eens opgeslagen in een tabel. Zo kan ik bepaalde ipadressen bijvoorbeeld ook blokkeren als iemand het eindeloos blijft proberen.
 
- Ariën  -
Beheerder

- Ariën -

28/05/2016 17:00:04
Quote Anchor link
J C op 28/05/2016 16:54:11:
Er is natuurlijk wel een verschil in iemand een script laten schrijven en iemand toegang geven tot een ftpserver.

Wie heeft het over ftp? ;-)

Verder hoeft een SQL-injection geen PHPerrors te werpen. Het ligt eraan hoe je logging inelkaar steekt.
Gewijzigd op 28/05/2016 17:01:49 door - Ariën -
 
Thomas van den Heuvel

Thomas van den Heuvel

28/05/2016 17:03:59
Quote Anchor link
Waar ik mee zou beginnen is één aanpak kiezen voor het aanspreken van je database.

In bovenstaande code gebruik je zowel de rechtstreekse manier om queries op te bouwen (op een onveilige manier) en vervolgens gebruik je prepared statements voor het loggen van fouten (zij het op een verkeerde manier, die ook onveilig is).

Persoonlijk vind ik prepared statements in MySQLi nogal bagger. Als je daar fan van bent kun je mogelijk beter PDO gebruiken. Maar ook dan geldt dat een onjuist gebruik onveilig is.

En dan moet de code in bovenstaand fragment nodig gerefactored worden. Deze kan vele malen korter. Hebben al die errorcodes echt nut en betekenis? Ik hoop dat je niet terugkoppelt aan een gebruiker wat er precies niet goed was aan de inlog, want dit maakt het inbreken in je site makkelijker.

De uitkomt van een inlogpoging is ofwel GOED ofwel FOUT, en verder niets.
 
J C

J C

28/05/2016 17:05:18
Quote Anchor link
Thomas had het over broncode. Terwijl die dat niet nodig is voor het schrijven van een inlogscript.

Ik heb het script nu zo aangepast, ik heb het nog niet gechecked, dus er kunnen nog wat foutjes inzitten :

Code (php)
PHP script in nieuw venster Selecteer het PHP script
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
62
63
64
65
66
67
<?php
$password
= $_POST['mw_pass'];
$mw_user = $_POST['mw_user'];
//check personal salt
$saltqry = $connection->query("
                                SELECT
                                        salt,
                                        mw_gegevens_groep                                        
                                FROM
                                        mw_gegevens
                                WHERE     
                                        mw_gegevens_persnr=?
                                "
);
        $statement = $connection->prepare($saltqry);
        if($qry === false)
        {

        echo "Query error:.". $connection->error();
        }

        else
        {     
            $statement->bind_param('ii', $mw_user, $userpassword);  
            $statement->execute();
            $result = $statement->get_result();    
            $satement->close;
            $saltuitkomt = $result->fetch_assoc();
        }
    (
$erroruitkomst = $saltqry->fetch_assoc());
    if($saltqry->num_rows == 0)
    {

        $salt='';
    }

    else
    {
        $salt = $saltuitkomst['salt'];
    }

    
    include_once('inlog/passcrypt.php');

    $userpassword = $Nieuw_ww;
    

    $mw_gegevens_qry = $connection->query("
                                SELECT
                                        *
                                FROM
                                        mw_gegevens
                                WHERE     
                                        mw_gegevens_persnr=?
                                AND     
                                        mw_gegevens_pass=?
                                AND
                                        mw_gegevens_pass!=''
                                "
);
        $statement = $connection->prepare($mw_gegevens_qry);
        if($qry === false)
        {

        echo "Query error:.". $connection->error();
        }

        else
        {     
            $statement->bind_param('ii', $mw_user, $userpassword);  
            $statement->execute();
            $result = $statement->get_result();    
            $satement->close;
            $mwgegevens = $result->fetch_assoc();
        }

?>


Toevoeging op 28/05/2016 17:06:44:

Thomas van den Heuvel op 28/05/2016 17:03:59:
Waar ik mee zou beginnen is één aanpak kiezen voor het aanspreken van je database.

In bovenstaande code gebruik je zowel de rechtstreekse manier om queries op te bouwen (op een onveilige manier) en vervolgens gebruik je prepared statements voor het loggen van fouten (zij het op een verkeerde manier, die ook onveilig is).

Persoonlijk vind ik prepared statements in MySQLi nogal bagger. Als je daar fan van bent kun je mogelijk beter PDO gebruiken. Maar ook dan geldt dat een onjuist gebruik onveilig is.

En dan moet de code in bovenstaand fragment nodig gerefactored worden. Deze kan vele malen korter. Hebben al die errorcodes echt nut en betekenis? Ik hoop dat je niet terugkoppelt aan een gebruiker wat er precies niet goed was aan de inlog, want dit maakt het inbreken in je site makkelijker.

De uitkomt van een inlogpoging is ofwel GOED ofwel FOUT, en verder niets.


Ik koppel terug dat een invulveld leeg is, of dat een account geblokkeerd is.
Gewijzigd op 28/05/2016 17:07:23 door J C
 
Thomas van den Heuvel

Thomas van den Heuvel

28/05/2016 17:20:49
Quote Anchor link
Quote:
of dat een account geblokkeerd is.

Dat is het ontsluiten van informatie over de toestand van een account wat mij niet verstandig lijkt.

Je zou eerder een algemene foutmelding kunnen geven als een loginpoging strandt. Hierbij zou je niet eens onderscheid te hoeven maken of iemand iets invult of niet, maar je zou in de afhandeling dan verdere controles geheel kunnen skippen (als een gebruikersnaam of wachtwoord leeg is hoef je niets te controleren omdat dat toch nooit wat oplevert).

Je zou in de algemene foutmelding kunnen opnemen dat als een inlogpoging herhaaldelijk mislukt dat iemand kan proberen zijn/haar wachtwoord op te vragen. Vervolgens zou je dan iemand persoonlijk vervolgstappen kunnen berichten. Hierin zou je ook kunnen aangeven dat iemand zijn account is geblokkeerd en wat deze persoon vervolgens kan doen.
 
J C

J C

28/05/2016 17:28:03
Quote Anchor link
Goed idee.

Ik kom dan op zoiets uit.
Het betreft hier onze medewerkers website. Als een medewerker niet meer in dienst is, dan wordt het account gebokkeerd.

Wat is precies het gevaar van die info delen? Ik zie dat zelf altijd wel als goede service.

Code (php)
PHP script in nieuw venster Selecteer het PHP script
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
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
<?php
if(isset($_POST['login']))
{

    if($_POST['mw_pass']=='' && $_POST['mw_user']=='')
    {

        $aErrors= 87;
        $user = '0';
    }

    elseif($_POST['mw_user']=='')
    {

        $aErrors=71;
        $user = '0';
    }

    elseif($_POST['mw_pass']=='')
    {

        $aErrors=8;
        $user = $_POST['mw_user'];
    }

    else
    {
        $password = $_POST['mw_pass'];
        $user = $_POST['mw_user'];
        //check personal salt
        $saltqry = $connection->query("
                                        SELECT
                                                salt,
                                                mw_gegevens_groep                                        
                                        FROM
                                                mw_gegevens
                                        WHERE     
                                                mw_gegevens_persnr=?
                                        "
);
                $statement = $connection->prepare($saltqry);
                if($qry === false)
                {

                echo "Query error:.". $connection->error();
                }

                else
                {     
                    $statement->bind_param('ii', $user, $userpassword);  
                    $statement->execute();
                    $result = $statement->get_result();    
                    $satement->close;
                    $saltuitkomt = $result->fetch_assoc();
                }
            (
$erroruitkomst = $saltqry->fetch_assoc());
            if($saltqry->num_rows == 0)
            {

                $salt='';
            }

            else
            {
                $salt = $saltuitkomst['salt'];
            }

            
            include_once('inlog/passcrypt.php');

            $userpassword = $Nieuw_ww;
    

    $mw_gegevens_qry = $connection->query("
                                SELECT
                                        *
                                FROM
                                        mw_gegevens
                                WHERE     
                                        mw_gegevens_persnr=?
                                AND     
                                        mw_gegevens_pass=?
                                AND
                                        mw_gegevens_pass!=''
                                "
);
        $statement = $connection->prepare($mw_gegevens_qry);
        if($qry === false)
        {

        echo "Query error:.". $connection->error();
        }

        else
        {     
            $statement->bind_param('ii', $user, $userpassword);  
            $statement->execute();
            $result = $statement->get_result();    
            $satement->close;
            $mwgegevens = $result->fetch_assoc();
        }

        if($mw_gegevens_qry->num_rows == 0)
        {

            $aErrors= 91;
        }

        elseif($saltqry->num_rows != 0 && $erroruitkomst['mw_gegevens_groep']==7)
        {

            $aErrors= 11;
        }

        elseif($saltqry->num_rows == 1)
        {

            $aErrors=0;
        }

        else
        {
            $aErrors=9999;
        }
    }

?>
Gewijzigd op 28/05/2016 17:28:32 door J C
 
Thomas van den Heuvel

Thomas van den Heuvel

28/05/2016 17:36:09
Quote Anchor link
Quote:
Wat is precies het gevaar van die info delen?

In dit concrete geval:
- een aanvaller weet dan dat dat een bestaand (zij het een gedeactiveerd) account betreft, waarmee deze een informatie-bestand kan opbouwen van accounts die mogelijk toegepast kan worden bij andere "ingangen" in je applicatie
- de aanvaller weet dan ook dat het op dat moment niet zinnig is om met dat account verdere loginpogingen te ondernemen

Ik zou eigenlijk alleen intern inlogpogingen bijhouden, en daarbij enkel het onderscheid maken tussen het slagen of falen hiervan en niet zozeer wat hier precies mis aan was, tenzij dit om een of andere reden heel belangrijk is. Anders is dit toch een beetje verspilde moeite.
 
J C

J C

28/05/2016 17:38:39
Quote Anchor link
OK daar heb je wel een goed punt.

Ik zal dan de fouten wel blijven opslaan, maar niet terugkoppelen naar de website.
Op de website maak ik dan wel een melding dat het inloggen niet gelukt is.
Gewijzigd op 28/05/2016 17:47:36 door J C
 



Overzicht Reageren

 
 

Om de gebruiksvriendelijkheid van onze website en diensten te optimaliseren maken wij gebruik van cookies. Deze cookies gebruiken wij voor functionaliteiten, analytische gegevens en marketing doeleinden. U vindt meer informatie in onze privacy statement.