SQL Injection
ik heb een probleem. ik heb een formulier waar ik via de get methode gegevens ophaal. alleen is deze gevoelig voor sql injectie. zoals hieronder te zien is gebruik ik mysql_real_escape_string strip_tags en htmlentities maar toch is het mogelijk om mijn database op te vragen. kan iemand mij helpen?
Groeten,
Rob
Code (php)
1
2
3
4
5
6
2
3
4
5
6
<?php
$Serie = mysql_real_escape_string(strip_tags(htmlentities($_GET["Serie"])));
$sql = mysql_real_escape_string(strip_tags(htmlentities("Select DISTINCT Seizoen From ". $Serie ." ORDER BY Seizoen")));
?>
$Serie = mysql_real_escape_string(strip_tags(htmlentities($_GET["Serie"])));
$sql = mysql_real_escape_string(strip_tags(htmlentities("Select DISTINCT Seizoen From ". $Serie ." ORDER BY Seizoen")));
?>
- Onnodig variabelen kopiëren kost tijd, gebruik gewoon de $_GET var + mysql_real_escape_string in je query.
- Mysql_real_escape_string is alleen nodig voor stings, niet voor getallen enz. Ook is het overbodig en misschien zelfs fout om het rond een query te zetten. Dit geld ook voor alle andere functies.
Wat bedoel je met 'maar toch is het mogelijk om mijn database op te vragen.'? Want dat kan zo namelijk niet meer...
Zoek de verschillen:
Code (php)
1
2
3
4
5
6
7
8
9
10
11
2
3
4
5
6
7
8
9
10
11
<?php
$_POST['input'] = '1 or 1=1';
// zo lek als een mandje:
$query = "SELECT * FROM tabel WHERE id = ".mysql_real_escape_string($_POST['input']);
echo $query;
// veilig
$query = "SELECT * FROM tabel WHERE id = '".mysql_real_escape_string($_POST['input'])."'";
echo $query;
?>
$_POST['input'] = '1 or 1=1';
// zo lek als een mandje:
$query = "SELECT * FROM tabel WHERE id = ".mysql_real_escape_string($_POST['input']);
echo $query;
// veilig
$query = "SELECT * FROM tabel WHERE id = '".mysql_real_escape_string($_POST['input'])."'";
echo $query;
?>
Ps. Wouter J slaat hier en daar de plank mis, er is meer nodig om veilig te werken.
Verder is je query niet goed, selecteer gewoon wat je wilt en niet * gebruiken.
Ook moet je bij getallen helemaal geen mysql_real_escape_string gebruiken. Bij getallen moet je gaan typecasten om je query veilig te maken.
En voordat we elkaar hier allemaal gaan uitmaken voor 'slecht' en 'hier en daar de plank mis slaan' wens ik je nog een vrolijk en gezegend kerstfeest toe.
Wat Bartje aanstipt is gewoon een feit. In zijn voorbeeld is mysql_real_escape_string($_POST['input']) zonder quotes er omheen gewoon lek.
En wat Wouter zegt (int) $_POST['input'] is in orde.
Wel een interessante discussie.
Gewijzigd op 25/12/2011 17:36:48 door - SanThe -
Wouter J op 25/12/2011 16:14:58:
Verder is je query niet goed, selecteer gewoon wat je wilt en niet * gebruiken.
De query is uitstekend, zit geen enkele fout in. Dat het gebruik van een * niet is aan te raden voor een productie systeem, dat is logisch. Maar voor een lullig voorbeeldje voldoet het uitstekend.
Quote:
Ook moet je bij getallen helemaal geen mysql_real_escape_string gebruiken.
Voor PHP ziet de hele wereld eruit als een string, dat is dus geen argument.
Quote:
En dat zou SQL injection tegen moeten gaan? SQL injection ga je tegen door de database aan het werk te zetten, die moet bekijken of de userinput een correct datatype heeft. mysql_real_escape_string() is dus al een lapmiddel, je hebt eigenlijk prepared statements nodig. Dan kun je ook in het logboek van jouw database zien welke queries veilig zijn en waar nog risico's zitten.Bij getallen moet je gaan typecasten om je query veilig te maken.
Met MySQL is het alleen een hoop geknutsel om met prepared statements te werken, je hebt er extra code voor nodig. Zowel met PDO als MySQLi is het behelpen, dit houdt vele programmeurs tegen, en dat is jammer.
Kijk ook eens hoe PHP dat voor PostgreSQL heeft opgelost, met pg_query_params() heb je maar één functie nodig om de query op te sturen en de userinput op een veilige manier mee te geven. Ook het fetchen blijft lekker eenvoudig, heb je geen extra code voor nodig. (zelfs nog minder code dan voor fetchen van een MySQL-resultaat)
Kortom, laat de database het werk voor jou doen, hoef jij niet meer aan te prutsen met typecasting e.d. Kun je leuke dingen gaan doen :-)
Wouter heeft zeer zeker gelijk wat betreft de * die kan je beter nooit gebruiken...
ook heeft hij gelijk wat betreft controleren van int met mysql_real_escape_string
Daarbij ben ik van mening dat mysql_real_escape_sting() enkel en alleen voor string waarde gebruikt dient te worden en niet voor Int waardes omdat dit gewoon geen zin heeft omdat je een int op een andere manier checkt...
Daarnaast is phpmyadmin niet de betrouwbaarste en stabielste database (tool) die er bestaad er gebeuren soms hele rare dingen bij hun...
Maar je kan de input van een user/gebruiker beter een keer teveel checken dan te weining en de input moeten nou eenmaal in goede banen geleid worden..
vertrouw NOOIT op de correctheid van de input de gebruiker
Vertrouw NOOIT op volledige correctheid van de input van programmeurs/bouwers..
Vertrouw Niemand dus check en check alles..
Bartje Jansen:
Kortom, laat de database het werk voor jou doen, hoef jij niet meer aan te prutsen met typecasting e.d. Kun je leuke dingen gaan doen :-)
Om met iets positiefs te beginnen. Hier ben ik het volledig mee eens. Prepared statements zijn het best te gebruiken, zelf script ik ook altijd met PDO want MySQLi heeft wat rare fratsen met prepared statements.
Bartje Jansen:
Voor PHP ziet de hele wereld eruit als een string, dat is dus geen argument.
Hier sla je helaas wel de plank mis.
Als gebruiker denk je dat alles eruit ziet als een string, maar het is niet zo. Omdat de webdevelopers wel eens de mist in gaan keurt PHP '212' ook goed en zet het zelf om naar 212. Het is niet zo dat PHP alles leest in strings.
Precies zoals dat 0 gezien wordt als false en elke andere waarde als true. Dat is ook zoiets dat in PHP is toegevoegd voor het gemak van de gebruiker en dat is niet zoiets dat hoort in PHP.
PHP is eigenlijk een totaal verkeerde taal. Het is wel leuk allemaal, maar zo soepel. Alles voor het gemak van de developer, maar eigenlijk zijn veel PHP scripts logisch gezien fout omdat PHP zo gemakkelijk omgaat met de regels. Nee, probeer maar eens in echte stricte talen dit soort code uit, je krijg dan schermen vol met errors...
@Marco en vertrouw ook niet jezelf. Zeg niet ik ben de enige die die variabele beheert dus daar hoeft geen real_escape_string over, want dan zit je fout. Je moet alles controleren.
Ik heb bij update/delete queries zelfs altijd een extra functie die kijkt of ik niet ergens een where vergeten ben, want ik wil mezelf checken en zeker weten dat ik niks fout doe.
Dat zeg ik ook en wilde ik ook totaal niet zeggen..
Ik wilde ermee zeggen dat je juist alles moet controleren..
Quote:
Je moet alles controleren.
Dat is juist wat ik zeg...