Gevaarlijk script
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
Post eens de relevante code, te beginnen met de in de stack trace genoemde bestanden rond de genoemde regels
Index.php is namelijk alleen een verzameling van includes en variabelen.
Het enige dat ik kan vinden zou dit kunnen zijn:
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
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
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;
}
}
?>
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
Gewijzigd op 28/05/2016 16:30:34 door - Ariën -
Is dit gewoon op te lossen met mysql_real_escape_string?
Want mysql_real_escape_string evenals dé andere soortgelijke mysql-functies zijn 'deprecated'
Ik gebruik mysqli, daarom dacht ik dat sql injectie niet meer mogelijk was.
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 -
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?
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.
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.
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 -
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 heb het script nu zo aangepast, ik heb het nog niet gechecked, dus er kunnen nog wat foutjes inzitten :
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
62
63
64
65
66
67
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();
}
?>
$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.
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
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.
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)
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
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;
}
}
?>
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
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.
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