Vraag over SQL-injectie (veilige GET-waarde)

Overzicht Reageren

Sponsored by: Vacatures door Monsterboard

The Ultimate

The Ultimate

08/09/2010 16:59:16
Quote Anchor link
Even een korte vraag. Is het volgende veilig tegen injecties?

Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
6
7
8
<?php
if(empty($_GET['dossierId']) || !is_numeric($_GET['dossierId']))
{

    $dossierId = 1;
}
else {
    $dossierId = $_GET['dossierId'];
}

?>


Zo niet, hoe kan ik dan veilig met een (numeriek) GET-waarde werken?
Gewijzigd op 08/09/2010 17:04:31 door The Ultimate
 
PHP hulp

PHP hulp

28/03/2024 14:20:20
 
Erwin Geen

Erwin Geen

08/09/2010 17:29:25
Quote Anchor link
Je kunt beter geen is_numeric gebruiken voor ID's, want die laat ook punten toe.
Je kunt (geloof ik) beter gebruik maken van ctype_digit.
 
Niek s

niek s

08/09/2010 17:53:02
Quote Anchor link
of intval :)

Edit:
Dat is dan ook rechtstreeks te gebruiken:

Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
<?php
$query
= "SELECT naam,bla FROM users WHERE id = " . intval($_GET['id']);
?>

Is gewoon veilig.

Edit 2:
Zelfs bij een lege string:
Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
niek@niek-desktop:/$ php -r 'echo intval("");'

Gewijzigd op 08/09/2010 17:56:02 door niek s
 
The Ultimate

The Ultimate

08/09/2010 18:16:58
Quote Anchor link
@Erwin Geen:
Klopt. ctype_digit zou een verbetering zijn.

@Niek s:
Bedankt! Die ga ik gebruiken. Zie op php.net nu ook dat dat een veilige oplossing is. Thanks a lot.
 
Pieter van Linschoten

Pieter van Linschoten

08/09/2010 19:08:23
Quote Anchor link
Voorbeeldje rechtstreeks van php.net:
Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
6
7
8
9
10
<?php
$numeric_string
= '42';
$integer        = 42;

ctype_digit($numeric_string);  // true
ctype_digit($integer);         // false

is_numeric($numeric_string);   // true
is_numeric($integer);          // true
?>


Verder:
Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
<?php
ctype_digit("-1");   //false
?>


En (niet heel erg boeiend):
Quote:
Using is_numeric function is quite faster than ctype_digit.
is_numeric took 0.237 Seconds for one million runs. while ctype_digit took 0.470 Seconds.


Mijn conclusie: Hoewel je bij is_numeric zoals eerder vermeld 1 punt (.) kunt gebruiken, brengt dat de veiligheid wat betreft je database niet in het geding. Ctype_int kijkt of een string bestaat uit een int. Aangezien je je database velden als INT opslaat, en je dus ook eventueel -1 getallen zou kunnen gebruiken, volstaat ctype_int niet altijd voor IDs, waar is_numeric weer net iets te coulant is.

Dus om je vraag te beantwoorden:
Quote:
Is het volgende veilig tegen injecties?

Ja.
Gewijzigd op 08/09/2010 19:13:48 door Pieter van Linschoten
 
Noppes Homeland

Noppes Homeland

08/09/2010 19:34:28
Quote Anchor link
en empty moet je ook eens verbannen uit je code!

empty is zinloos, je controleerd namenlijk niet of het een lege waarde is.
http://www.php.net/empty

En vergeet vooral niet eerst met isset te kijken of de variabele wel bestaat en daarna uiteraard de validatie. valideren heeft geen zin op iets wat niet bestaat.

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
<?php
if (   isset($_GET['dossierId']
    &&
ctype_digit($_GET['dossierId'])) {

   $sql = "....";
}


//als het om een string gaat
if (   isset($_GET['dossieromschrijving']
    &&
trim($_GET['dossieromschrijving']) == '') {

   $sql = "....";
}

/*
of je kijkt naar de mogelijkheden van
- http://www.php.net/ctype
- http://www.php.net/preg_match
- http://www.php.net/strlen
enz
*/

?>


En ook zeer zeker niet direct casten met intval of (int)

SQL injectie ga je tegen door:
_real_escape_string
of
bind_params gebruiken
 
The Ultimate

The Ultimate

08/09/2010 20:29:16
Quote Anchor link
@Pieter van Linschoten:
Thanks, duidelijke uitleg!

@Noppes Homeland:
Voor wat betreft dat empty gebeuren heb je in dit geval gelijk. Maar hoe zit het dan in dit voorbeeld:

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
<?php

$string
= array();

if(empty($string)){

    echo 'De string is empty';

}


if(!isset($string)){

    echo 'De string is niet gezet';

}


?>


Bedankt voor de verdere toelichting alvast!

EDIT:
Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
<?php
//als het om een string gaat
if (   isset($_GET['dossieromschrijving']
    &&
trim($_GET['dossieromschrijving']) == '') {
?>

Volgens mij klopt dit niet helemaal. Je kijkt nu of de GET['dossieromschrijving'] leeg is?

EDIT 2:
Ik denk toch dat ctype_digit mijn voorkeur krijgt aangezien intval ingeval van 'false' de waarde 1 teruggeeft.
Gewijzigd op 08/09/2010 20:41:58 door The Ultimate
 
Marco van Oort

Marco van Oort

08/09/2010 21:12:49
Quote Anchor link
Geeft intval in het geval 'false' 1 terug of in het geval FALSE?
Bij beide hoort het namelijk 0 terug te keren volgens mij.


@Noppes: Waarom zou je empty verbannen? Het is toch gewoon volwaardige functie?
 
The Ultimate

The Ultimate

08/09/2010 21:25:48
Quote Anchor link
@Marco:
intval geeft in het geval van 'false' de waarde 1 terug. Erg onhandig dus als je controleert op ID. Er is namelijk altijd wel een id=1 te vinden in de database.
 
Marco van Oort

Marco van Oort

09/09/2010 07:23:29
Quote Anchor link
Ja inderdaad, vrij logisch eigenlijk. Toch een is_numeric() oid nodig dan.
 
Johan Dam

Johan Dam

09/09/2010 09:45:07
Quote Anchor link
Persoonlijk zou ik inderdaad controleren op ids met bv ctype_digit, dit omdat je dan kan zien dat iemand iets aan het proberen is,

Als een bepaald IP mysql probeert te injecteren dan is het handig om dit te weten.

Empty is een handige functie, 0 is een nummer, welke functie je ook gebruikt, maar toch is het geen valid id, empty ziet het als leeg dus word hij niet toegelaten. zelfde geld voor false.

Je moet zeker oppassen met empty, je kan in sommige situaties genoeg 'rare fouten' krijgen als je niet oplet, maar in dit geval is het een hele handige functie.
 
Niek s

niek s

09/09/2010 13:53:10
Quote Anchor link
The Ultimate op 08/09/2010 21:25:48:
@Marco:
intval geeft in het geval van 'false' de waarde 1 terug. Erg onhandig dus als je controleert op ID. Er is namelijk altijd wel een id=1 te vinden in de database.


nou en?
Als het een enorm veiligheids lek is in een dergelijk geval, dan zitten er meerdere ontwerp fouten in je systeem ;)
 
The Ultimate

The Ultimate

09/09/2010 16:14:32
Quote Anchor link
@niek s:
Je hebt gelijk, het maakt ook eigenlijk geen zak uit. Ik dacht dat het van belang was omdat ik de dossierId ook gebruik om te controleren of een gebruiker een bepaald dossier mag inzien. Dat heeft dus geen effect want als dat '1' is dan controleer ik alsnog of de gebruiker daadwerkelijk de 'eigenaar' van het dossier is.

My bad..

EDIT:
Om nog even terug te komen op Noppes:
Hoe controleer jij dan of een bepaald veld van een formulier ingevuld is?
Ik doe dat dmv empty($_POST['veld']). Met isset() gaat je dit niet lukken.
Gewijzigd op 09/09/2010 16:18:38 door The Ultimate
 
Dalando De Zuil

Dalando De Zuil

09/09/2010 16:33:44
Quote Anchor link
The Ultimate op 09/09/2010 16:14:32:
EDIT:
Om nog even terug te komen op Noppes:
Hoe controleer jij dan of een bepaald veld van een formulier ingevuld is?
Ik doe dat dmv empty($_POST['veld']). Met isset() gaat je dit niet lukken.


if($_SERVER['REQUEST_METHOD'] == "POST / GET") ?
Gewijzigd op 09/09/2010 16:34:01 door Dalando De Zuil
 
- Raoul -

- Raoul -

09/09/2010 16:44:59
Quote Anchor link
Dalando das om te kijken of het formpje GEPOST is.
En empty(); moet wel lukken... toch??
 
The Ultimate

The Ultimate

09/09/2010 16:56:42
Quote Anchor link
@Dalando:
Dat is inderdaad om te controleren of het formulier is verzonden. Daarmee controleer je niet de waarde van een bepaald veld.

@Internet Verslaafde:
Ja, dat zeg ik net....
 
Niek s

niek s

09/09/2010 17:25:48
Quote Anchor link
Let niet op Dalando, die weet niet waar hij over praat.

OT:
Het ligt er een beetje aan wat je wilt. Zoals hierboven beschreven staat, kan een test gebruikt worden. Dus een test om te zien of het veld numeriek is. Dit kan je bijvoorbeeld doen om te controleren of het valide is, en zo niet een foutmelding te geven en eventueel wat te loggen.
Als je denkt "het zal allemaal wel", kun je ook intval gebruiken. Het gebruik van intval zou niet af mogen hangen van een correcte werking van je project. Laat dit dus geen argument zijn. Als dat wel is, moet je eens je code nalopen wat er verder niet klopt.

Hoe test je of een bepaalde leeg is?
Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
4
5
if(isset($_POST['bla']) && !empty($_POST['bla'])) {
    // goed
} else {
    // fout
}


Quote:
Ja maar waarom werd er dan gezegd dat empty slecht is ? (ofzo)

Dat is om de volgende reden, zie citaat PHP.net:
Quote:
Returns FALSE if var has a non-empty and non-zero value.

The following things are considered to be empty:

* "" (an empty string)
* 0 (0 as an integer)
* "0" (0 as a string)
* NULL
* FALSE
* array() (an empty array)
* var $var; (a variable declared, but without a value in a class)

Kortom: precies wat je wil dus.
De reden dat empty true terug geeft bij al die waardes is omdat daar over nagedacht is ;-)
 
The Ultimate

The Ultimate

09/09/2010 17:44:38
Quote Anchor link
@niek s:
Dank u. Ik controleer ook eigenlijk altijd op de wijze die jij hierboven beschrijft (dus isset en empty). Vreemd dat Noppes reageert door te stellen dat je 'empty' uit je code zou moeten verbannen. Ik vind 'empty' juist een hele fijne functie.

Wat betreft die Dalando (moest trouwens lachen om jouw reactie): die kan of niet lezen, of hij is zijn stats aan het boosten. Geldt ook voor die Internet Verslaafde overigens.

Thanks again!
 
Mark L

Mark L

09/09/2010 19:12:03
Quote Anchor link
Je zou ook dit kunnen doen.
Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
<?php
$query
= mysql_query("SELECT name FROM users WHERE id = ".(int)$_GET['id']);
?>


Je zorgt niet voor een controle (en kunt dus geen IP adressen 'flaggen'), maar SQL-injection is onmogelijk.
 
Niek s

niek s

09/09/2010 20:13:19
Quote Anchor link
Mark L op 09/09/2010 19:12:03:
Je zou ook dit kunnen doen.
Code (php)
PHP script in nieuw venster Selecteer het PHP script
1
2
3
<?php
$query
= mysql_query("SELECT name FROM users WHERE id = ".(int)$_GET['id']);
?>


Je zorgt niet voor een controle (en kunt dus geen IP adressen 'flaggen'), maar SQL-injection is onmogelijk.


Typecasten is nagenoeg hetzelfde als de *val() functies, zoals de intval() die ik hierboven een aantal keer noem.
 



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.