Niek's lijstje
Hem een PMpje gestuurd, en toen zei hij dat ik het op het forum moet zetten.
Dus dit waren zijn punten:
Quote:
login.php,
- regel 14. Daar kijk je niet of die POST-shit geset is ja/nee. Doe ook even een isset()
- regel 20. In principe is een real_escape_string niet nodig op een md5 hash, maar goed. Kan ook geen kwaad.
- regel 26. Waarom kies je er soms voor om accolade's op dezelfde regel te zetten, en andere keren op de volgende regel? (en dan nog op het verkeerde niveau ingesprongen ook)
- regel 20. eigenlijk nooit * gebruiken in een query.
register.php
- regel 7. Vind die melding vaag. je moet registreren maar dat ben je al?
- regel 15. Wederom, check ook even isset()
- regel 38. Waarom check je op die manier? Je kan de username ook UNIQUE maken, en vervolgens de errno afvangen. Op deze manier laat je namelijk je database je data beheren. Het is niet voor niets je database ;-)
- regel 65. Zelfde verhaal weer over die md5/escape shit
- regel 70. Typo.
- regel 71. Typo.
- regel 72. Mail Injection mogelijk.
functions.php
- Je regular expression klopt volgensmij niet. Ik zal die eens testen:
- regel 14. Daar kijk je niet of die POST-shit geset is ja/nee. Doe ook even een isset()
- regel 20. In principe is een real_escape_string niet nodig op een md5 hash, maar goed. Kan ook geen kwaad.
- regel 26. Waarom kies je er soms voor om accolade's op dezelfde regel te zetten, en andere keren op de volgende regel? (en dan nog op het verkeerde niveau ingesprongen ook)
- regel 20. eigenlijk nooit * gebruiken in een query.
register.php
- regel 7. Vind die melding vaag. je moet registreren maar dat ben je al?
- regel 15. Wederom, check ook even isset()
- regel 38. Waarom check je op die manier? Je kan de username ook UNIQUE maken, en vervolgens de errno afvangen. Op deze manier laat je namelijk je database je data beheren. Het is niet voor niets je database ;-)
- regel 65. Zelfde verhaal weer over die md5/escape shit
- regel 70. Typo.
- regel 71. Typo.
- regel 72. Mail Injection mogelijk.
functions.php
- Je regular expression klopt volgensmij niet. Ik zal die eens testen:
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
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
niek@niek-desktop:~$ cat faal.php
<?php
function validate_email($email)
{
$email = trim( $email );
if(!empty($email)){
if(preg_match('/^[a-z0-9\\_\\.]+@[a-z0-9\\-]+\\.[a-z]+\\.?[a-z]{2,4}$/i', $email, $match)){
return strtolower($match[0]);
}
}
return false;
}
echo validate_email($argv[1]) ? 'OK' : 'FAAL';
echo '\n';
?>
niek@niek-desktop:~$ php faal.php [email protected]
FAAL
niek@niek-desktop:~$ php faal.php [email protected]
OK
niek@niek-desktop:~$ php faal.php [email protected]
FAAL
niek@niek-desktop:~$ php faal.php [email protected]
OK
niek@niek-desktop:~$ php faal.php [email protected]
FAAL
<?php
function validate_email($email)
{
$email = trim( $email );
if(!empty($email)){
if(preg_match('/^[a-z0-9\\_\\.]+@[a-z0-9\\-]+\\.[a-z]+\\.?[a-z]{2,4}$/i', $email, $match)){
return strtolower($match[0]);
}
}
return false;
}
echo validate_email($argv[1]) ? 'OK' : 'FAAL';
echo '\n';
?>
niek@niek-desktop:~$ php faal.php [email protected]
FAAL
niek@niek-desktop:~$ php faal.php [email protected]
OK
niek@niek-desktop:~$ php faal.php [email protected]
FAAL
niek@niek-desktop:~$ php faal.php [email protected]
OK
niek@niek-desktop:~$ php faal.php [email protected]
FAAL
En hier mijn vraagjes:
Login.php
Regel 14:
Hoe moet ik dat doen met die isset?
Moet ik daar een aparte if voor maken? Of in die if houden? Moet ik dan || of && gebruiken?
moet het dus zo:
Code (php)
1
if(!isset($_POST['username']) || !isset($_POST['password']) || empty($_POST['username']) || empty($_POST['password'])){
of moet het zo?
Code (php)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
if(isset($_POST['username']) || isset($_POST['password'])){
if(empty($_POST['username']) || empty($_POST['password'])) {
echo 'Password or username was empty! Please try again in 5 seconds.';
header('Refresh: 5; url=login.php');
exit;
}
else
{
// Doorgaan: username & password waren gezet en ze waren niet leeg.
}
}
else
{
echo 'Username or password was not set! Please try again in 5 seconds.';
header('Refresh: 5; url=login.php');
exit;
}
if(empty($_POST['username']) || empty($_POST['password'])) {
echo 'Password or username was empty! Please try again in 5 seconds.';
header('Refresh: 5; url=login.php');
exit;
}
else
{
// Doorgaan: username & password waren gezet en ze waren niet leeg.
}
}
else
{
echo 'Username or password was not set! Please try again in 5 seconds.';
header('Refresh: 5; url=login.php');
exit;
}
Regel 26
Ik snap de vraag niet..? Ik zie niks mis ermee hoor?
Register.php
Regel 7
Ik check daar of de gebruiker al ingelogd is.
Regel 15
Zelfde vraag als bij Login.php regel 14.
Regel 38
Ik zou niet weten hoe ik het anders moest afhandelen.
Regel 72
Niet waar, ivm htmlentities().
Functions.php
Dat is nog even te lastig voor mij, dus dat scriptje had ik van het internet gehaald.
Ik zal zoeken naar een nieuwe, en zo nodig hier laten keuren.
Aan de rest ga ik werken, tot zover ik kan.
Groetjes Dalando :)
Toevoeging op 25/09/2010 16:29:46:
En een mail checker:
Code (php)
is dit een goeie?
Gewijzigd op 25/09/2010 16:30:34 door Dalando De Zuil
if( isset($_POST['username']) && !empty($_POST['wachtwoord']) {
// goed
}
Regel 26
je deed zoiets:
Code (php)
1
2
3
2
3
if(....){
while(....)
{
while(....)
{
die accolade staat op de hoogte van de if, en op de volgende regel. Terwijl je bij de if het op dezelfde regel hebt. Dat is gewoon vaag.
Regel 7: dat snap ik, maar die melding is vaag. Lees hem zelf eens. "Je moet registreren maar dat ben je al" (oid), dat staat er.
Regel 38: dat staat er bij!
Regel 72: Sinds wanneer doet htmlentities iets met enters? Nou is dat op die plaats ook niet relevant, ik had het eigenlijk over je email adres. En de enige reden dat ik die opmerking plaatste was eigenlijk omdat je regex niet klopt. Als je een kloppende regex hebt, kan er ook geen mail injection meer op die plaats. ;)
Edit:
Code (php)
1
2
3
4
5
6
2
3
4
5
6
$filtered_email = filter_var($email, FILTER_VALIDATE_EMAIL);
if ($filtered_email !== false) {
return TRUE;
} else {
return FALSE;
}
if ($filtered_email !== false) {
return TRUE;
} else {
return FALSE;
}
Nee dat is geen goede.
Als ik het goed heb onthouden las ik laatst ergens dat de interne regex van filter_var niet helemaal juist is. (Karl?)
Verder is wat je in die if doet een beetje kansloos. Heel die functie kan op 1 regel, en dus is het nut van die functie eigenlijk weg ;) Een string is namelijk in PHP hetzeflde als een boolean true.
function bla($email) {
return filter_var($email, FILTER_VALIDATE_EMAIL);
}
Gewijzigd op 25/09/2010 16:34:27 door niek s
Maar nog een paar vraagjes:
Register.php 38: "Je kan de username ook UNIQUE maken, en vervolgens de errno afvangen"
Dat is dus even chinees voor mij :S
Register.php 72: "Sinds wanneer doet htmlentities iets met enters?"
Hoe kan er dan met enters een injection ontstaan, en hoe kan ik ze voorkomen?
Dalando De Zuil op 25/09/2010 16:44:33:
Right. Voor dat 1.4 de lucht in gaat, laat ik de login.php, functions.php & register.php nog even controleren... De rest blijft allemaal hetzelfde.
Maar nog een paar vraagjes:
Register.php 38: "Je kan de username ook UNIQUE maken, en vervolgens de errno afvangen"
Dat is dus even chinees voor mij :S
Register.php 72: "Sinds wanneer doet htmlentities iets met enters?"
Hoe kan er dan met enters een injection ontstaan, en hoe kan ik ze voorkomen?
Maar nog een paar vraagjes:
Register.php 38: "Je kan de username ook UNIQUE maken, en vervolgens de errno afvangen"
Dat is dus even chinees voor mij :S
Register.php 72: "Sinds wanneer doet htmlentities iets met enters?"
Hoe kan er dan met enters een injection ontstaan, en hoe kan ik ze voorkomen?
Als je op google zoekt naar UNIQUE, leest hoe dat werkt, en vervolgens kijkt wat de functie mysql_errno() doet in PHP moet je toch een eind kunnen komen ;-)
Over die mail shit: zoek eens op hoe mail werkt, en dan vooral de headers. Daar moet je wel kunnen uithalen wat er bedoeld wordt.
En mysql_errno geeft dus het mysql error id terug... Wat moet ik daarmee doen?
Gewijzigd op 25/09/2010 17:03:09 door Dalando De Zuil
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
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
$query = "SELECT * FROM dalanuser WHERE username = '". mysql_real_escape_string($_POST['username']) ."' AND password = '". mysql_real_escape_string(md5($_POST['password'])) ."'";
$mysqlquery = mysql_query($query);
$mysqlrows = mysql_num_rows($mysqlquery);
if($mysqlrows > 0){
while($row = mysql_fetch_assoc($mysqlquery)){
$_SESSION['dalansession'] = TRUE;
$_SESSION['dalanuser'] = $row['username'];
$_SESSION['dalanid'] = $row['userid'];
$_SESSION['dalanemail'] = $row['email'];
echo 'Welcome back, '. $row['username'] .'!';
header('Refresh: 5; url='.$settings['urllogin']);
exit;
}
}
else
{
echo 'Your username / password does not exist! Please try again, after 5 seconds.';
header('Refresh: 5; url=login.php');
exit;
}
$mysqlquery = mysql_query($query);
$mysqlrows = mysql_num_rows($mysqlquery);
if($mysqlrows > 0){
while($row = mysql_fetch_assoc($mysqlquery)){
$_SESSION['dalansession'] = TRUE;
$_SESSION['dalanuser'] = $row['username'];
$_SESSION['dalanid'] = $row['userid'];
$_SESSION['dalanemail'] = $row['email'];
echo 'Welcome back, '. $row['username'] .'!';
header('Refresh: 5; url='.$settings['urllogin']);
exit;
}
}
else
{
echo 'Your username / password does not exist! Please try again, after 5 seconds.';
header('Refresh: 5; url=login.php');
exit;
}
Maar dit geeft telkens: username / password does not exist! Please try again, after 5 seconds.
Wat is er fout?
Dat weet ik niet, want er ontbreekt fout afhandeling ;)
Synaps Framework op 25/09/2010 17:06:46:
Unique = Uniek. Succes
ja héhé.. Daar was ik nog niet achter. Maar wat genereerd het ?
Niek s op 25/09/2010 17:23:57:
Wat er fout is?
Dat weet ik niet, want er ontbreekt fout afhandeling ;)
Dat weet ik niet, want er ontbreekt fout afhandeling ;)
nou heb ik dit gedaan:
Code (php)
1
2
2
echo 'Your username / password does not exist! Please try again, after 5 seconds.';
echo trigger_error(mysql_error());
echo trigger_error(mysql_error());
krijg ik dit:
Notice: in C:\data\home\www\Projects\dalanuser\login.php on line 42
1
Niemand heeft hier zin om het google werk voor jou op te knappen. Zoek zelf eens wat.
Wat een UNIQUE veld is in SQL moet je wel kunnen vinden.
Wat je dus wil doen, is:
maak veld uniek
gooi data er in
"hey bestaat al" zegt SQL
dan zeg jij: "mooi, dan tief ik een foutmelding naar de user"
En waarom het nu fout gaat:
- echo je query eens?
- gooi eens alelen een fout op als er iets fout gaat
- staat de data wel in je database?
- stond je caps niet aan?
- etc etc etc
Gewijzigd op 25/09/2010 17:49:29 door niek s
Quote:
- stond je caps niet aan?
Nu moet ik lachen...
@Dalando, gebruik if / else structuur om foutmeldingen op te vangen..
Niels Kieviet op 25/09/2010 18:22:07:
Nu moet ik lachen...
@Dalando, gebruik if / else structuur om foutmeldingen op te vangen..
Quote:
- stond je caps niet aan?
Nu moet ik lachen...
@Dalando, gebruik if / else structuur om foutmeldingen op te vangen..
Gedaan, en kijk eens aan:
Code (php)
1
2
3
4
5
6
7
8
9
10
11
12
2
3
4
5
6
7
8
9
10
11
12
$query = "SELECT * FROM dalanuser WHERE username = '". mysql_real_escape_string($_POST['username']) ."' AND password = '". mysql_real_escape_string(md5($_POST['password'])) ."'";
$mysqlquery = mysql_query($query);
$mysqlrows = mysql_num_rows($mysqlquery);
if($mysqlrows){
echo 'Tellen is mogelijk';
}
else
{
echo 'Tellen is NIET mogelijk';
}
$mysqlquery = mysql_query($query);
$mysqlrows = mysql_num_rows($mysqlquery);
if($mysqlrows){
echo 'Tellen is mogelijk';
}
else
{
echo 'Tellen is NIET mogelijk';
}
En wat geeft het terug: Tellen is NIET mogelijk
Maar wat het probleem is: ??
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
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
<?php
session_start();
require_once 'mysql_config.php';
if(isset($_SESSION['dalansession']) && $_SESSION['dalansession'] == TRUE){
echo 'No need to register, you're already registered!<br/>Redirect in 5 seconds.';
header('Refresh: 5; url='.$settings['urllogin']);
exit;
}
// Check if submitted
if($_SERVER['REQUEST_METHOD'] == "POST"){
if(isset($_POST['username']) && !empty($_POST['wachtwoord'])){
echo 'One of the fields was empty! Please try again, in 5 seconds.';
header('Refresh: 5; url=login.php');
exit;
}
$query = "SELECT * FROM dalanuser WHERE username = '". mysql_real_escape_string($_POST['username']) ."' AND password = '". mysql_real_escape_string(md5($_POST['password'])) ."'";
$mysqlquery = mysql_query($query);
$mysqlrows = mysql_num_rows($mysqlquery);
if($mysqlrows){
echo 'Tellen is mogelijk';
}
else
{
echo 'Tellen is NIET mogelijk';
}
if($mysqlrows > 0){
while($row = mysql_fetch_assoc($mysqlquery)){
$_SESSION['dalansession'] = TRUE;
$_SESSION['dalanuser'] = $row['username'];
$_SESSION['dalanid'] = $row['userid'];
$_SESSION['dalanemail'] = $row['email'];
echo 'Welcome back, '. $row['username'] .'!';
header('Refresh: 5; url='.$settings['urllogin']);
exit;
}
}
else
{
echo 'Your username / password does not exist! Please try again, after 5 seconds.';
header('Refresh: 5; url=login.php');
exit;
}
}
?>
<form action="<?php echo $_SERVER['PHP_SELF']; ?>" method="post">
Username: <input type="text" name="username"/><br/>
Password: <input type="password" name="password"/><br/>
<br/>
<input type="submit" value="Login!"/><br/>
</form>
session_start();
require_once 'mysql_config.php';
if(isset($_SESSION['dalansession']) && $_SESSION['dalansession'] == TRUE){
echo 'No need to register, you're already registered!<br/>Redirect in 5 seconds.';
header('Refresh: 5; url='.$settings['urllogin']);
exit;
}
// Check if submitted
if($_SERVER['REQUEST_METHOD'] == "POST"){
if(isset($_POST['username']) && !empty($_POST['wachtwoord'])){
echo 'One of the fields was empty! Please try again, in 5 seconds.';
header('Refresh: 5; url=login.php');
exit;
}
$query = "SELECT * FROM dalanuser WHERE username = '". mysql_real_escape_string($_POST['username']) ."' AND password = '". mysql_real_escape_string(md5($_POST['password'])) ."'";
$mysqlquery = mysql_query($query);
$mysqlrows = mysql_num_rows($mysqlquery);
if($mysqlrows){
echo 'Tellen is mogelijk';
}
else
{
echo 'Tellen is NIET mogelijk';
}
if($mysqlrows > 0){
while($row = mysql_fetch_assoc($mysqlquery)){
$_SESSION['dalansession'] = TRUE;
$_SESSION['dalanuser'] = $row['username'];
$_SESSION['dalanid'] = $row['userid'];
$_SESSION['dalanemail'] = $row['email'];
echo 'Welcome back, '. $row['username'] .'!';
header('Refresh: 5; url='.$settings['urllogin']);
exit;
}
}
else
{
echo 'Your username / password does not exist! Please try again, after 5 seconds.';
header('Refresh: 5; url=login.php');
exit;
}
}
?>
<form action="<?php echo $_SERVER['PHP_SELF']; ?>" method="post">
Username: <input type="text" name="username"/><br/>
Password: <input type="password" name="password"/><br/>
<br/>
<input type="submit" value="Login!"/><br/>
</form>
$mysqlquery:
resource(5) of type (mysql result)
$mysqlrows:
int(0)
Toevoeging op 25/09/2010 21:27:14:
Synaps Framework op 25/09/2010 21:26:35:
Volgens mij telt die functie het aantal rijen, dus met if($mysqlrows){ heb je alsnog een kut afhandeling.
Hoe zou ik het dan moeten doen?
Code (php)
1
if(isset($_POST['username']) && !empty($_POST['wachtwoord'])){
LOL: tellen kan mogelijk zijn ;) 0 is ook false in een if.
check het resultaat van je query eens, en echo je query eens...
Gewijzigd op 25/09/2010 21:28:45 door niek s
De rest kun je zelf.
Synaps Framework op 25/09/2010 21:31:12:
if( variable waarde is groter dan nul ) {
De rest kun je zelf.
De rest kun je zelf.
Oh, je bedoeld wat bij hem op regel 33 staat?
Eerst lezen ajb ;-)
Dalando: Zie mijn vorige post.
Niek s op 25/09/2010 22:13:54:
Oh, je bedoeld wat bij hem op regel 33 staat?
Eerst lezen ajb ;-)
Dalando: Zie mijn vorige post.
Synaps Framework op 25/09/2010 21:31:12:
if( variable waarde is groter dan nul ) {
De rest kun je zelf.
De rest kun je zelf.
Oh, je bedoeld wat bij hem op regel 33 staat?
Eerst lezen ajb ;-)
Dalando: Zie mijn vorige post.
Nee dat bedoel ik niet.
Synaps Framework op 25/09/2010 22:20:48:
Nee dat bedoel ik niet.
Niek s op 25/09/2010 22:13:54:
Oh, je bedoeld wat bij hem op regel 33 staat?
Eerst lezen ajb ;-)
Dalando: Zie mijn vorige post.
Synaps Framework op 25/09/2010 21:31:12:
if( variable waarde is groter dan nul ) {
De rest kun je zelf.
De rest kun je zelf.
Oh, je bedoeld wat bij hem op regel 33 staat?
Eerst lezen ajb ;-)
Dalando: Zie mijn vorige post.
Nee dat bedoel ik niet.
Wat dan wel?