SQL-injectie bij numerieke ID's

Het antwoord op de eerste vraag stond direct bovenaan bij Google :

https://stackoverflow.com/questions/5052932/how-to-get-int-instead-string-from-form

Hier lopen een aantal dingen door elkaar denk ik, ook in het artikel waar @Adoptive naar linkt, want daar gaat het meer over het type variabele, en niet zozeer over het formaat of security.

Allereerst moet je je realiseren dat het hier om twee verschillende dingen gaat:
* het veilig opnemen van USER DATA in een query
Je wilt dat je queries veilig zijn.

* validatie van de USER DATA
Je wilt dat wat je in je database stopt ook voldoet aan bepaalde regels.

Deze twee dingen dienen verschillende doelen en zul je dus ook in afzondering moeten behandelen.

is (int)$_GET['id'] voldoende of niet

Dit is niet echt rechtstreeks te beantwoorden. Ja, het is voldoende in die zin dat een typecast ervoor zorgt dat een hele hoop rotzooi (en mogelijk alle?) uit $_GET['id'] wordt verwijderd. Nee, omdat er mogelijk nog steeds de grootst mogelijke onzin in staat, zo zal, als $_GET['id'] "aap" bevat, deze omgezet worden naar de integer 0.

Een typecast, of alles wat een poging doet om de invoer te repareren -en daarmee ook om te zetten- is wat mij betreft ongeschikt omdat dit mogelijk recht probeert te buigen wat krom is. Het is zaak dat je een validatie-stap hebt voordat je data wegschrijft. Voldoet de invoer niet, dan is deze gewoon fout en geef je deze terug aan de gebruiker om deze te repareren.

Het gebruik van een whitelist is aan te bevelen als de invoer slechts één of enkele waarden zou mogen hebben. Dit is dan onderdeel van je validatie-routine waarbij je kijkt of de waarde voorkomt in een lijst van toegestane waarden.

Voor de goede orde, als $_GET['id'] bijvoorbeeld refereert aan het id van een record in de database, zou op het bestaan van het record gecontroleerd moeten worden. En het inserten van nieuwe data gekoppeld aan dit record (of het updaten van de data van dit record zelf) zou in een transactie moeten zitten waarin het record waaraan gerefereerd wordt gelocked zou moeten worden. Zodat deze hele transactie één ondeelbaar geheel vormt.

Het belangrijkste bij het veilig maken van je queries lijkt mij dat je altijd consequent dezelfde methode toepast. Als je bepaalde data wel escaped met real_escape_string() (in combinatie met quotes, het een is niet veilig zonder het ander!) en andere niet, dan werkt dat verwarrend. Immers, wat betekent het dat een bepaald stuk data niet ge-escaped is? Betekent dit dat dit niet nodig was, of dat het toch vergeten was? Dit werkt gewoon verwarrend. Escape gewoon altijd en overal. Ook al is het niet direct nodig. Dit zorgt voor een consistente omgang met externe (user) data, waarbij je niet bij elk item een afweging hoeft te maken of het wel of niet nodig is om te escapen.

Maar validatie en het veilig maken van queries zijn dus twee compleet verschillende dingen...

thomas, Bedankt voor het uitgebreide antwoord. Er is inderdaad ook wel een validatie
id: start met is_numeric dan (int) en dan bestaat het id wel

waarde indien niet in lijst ==> verworpen

Ik zal dus ook nog escapen.

Gewoon bind-vars gebruiken. Hoef je helemaal niet na te denken/escapen.

Overigens een tipje voor dit soort checks: je kunt ook een array binnen krijgen: index.php?id[]=5. In de $_GET heb je nu een array met een entry met waarde 5. Omdat checks hier vaak niet op rekenen kun je soms toch net even verder komen dan bedoelt (of in ieder geval de boel goed in de soep laten lopen).

is_numeric() is veel te breed (accepteert ook octale en hexadecimale getallen of getallen met een exponent) en garandeert ook niet dat dit aan een bestaand record refereert.

Als dit onderdeel uitmaakt van een administratief systeem zou ik gewoon maatregelen treffen zodat de data in dit systeem kloppend is en kloppend blijft.

<?php

if (ctype_digit($_GET['id']) === true) {
// doe je ding
}

?>

Persoonlijk gebruik ik een regexp die kijkt of het een auto-increment id betreft, dat lijkt mij ook uiteindelijk precies hetgene wat je wilt weten.

<?php
function isIndex($in) {
    return preg_match('#^[1-9][0-9]*$#', $in) === 1;
}
?>

Vervolgens is het ook zaak om te controleren of dit een bestaand record (en/of een record waar iemand toegang toe heeft!) betreft. Dit regel je in een transactie met FOR UPDATE.

>> Persoonlijk gebruik ik een regexp die kijkt of het een auto-increment id betreft, dat lijkt mij ook uiteindelijk precies hetgene wat je wilt weten.

Waarom niet ctype_digit?

Jan R op 23/08/2018 17:02:37

Als ik via GET nummers meegeef en ik doe natuurlijk ook controle of het wel nummers zijn, moet ik dan nog werken met mysqli_real_escape_string?

Nee, na een controle op nummers is escapen helemaal nergens voor nodig …

Jan R op 23/08/2018 17:02:37

is (int)$_GET['id'] voldoende of niet

… en ja, is een type cast inderdaad voldoende.

>> … en ja, is een type cast inderdaad voldoende.

Behalve dan dat ook niet-geldige invoer wordt omgezet naar een getal, zoals Thomas in z'n eerste post uitlegt. Je doet in dit geval dus niet een controle of het wel of niet een getal is, maar je zet alles om naar een getal.