Door
Jan R
op 23-08-2018 17:02
gewijzigd op 24-08-2018 07:44
6.292 views
hi,
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?
is (int)$_GET['id'] voldoende of niet
Eveneens gelinkt aan deze vraag; de 2° parameter is een string van 1 karakter(slechts 7 mogelijkheden) opgevangen door een switch en omgezet naar de juiste databasestring van ook 1 karakter. Zelfde vraag als hierboven.
Ongeldige invoer wordt niet omgezet naar "een getal" maar naar de integer 0. Hoewel dat misschien tegenintuïtief lijkt, is 0 géén geldige AUTO_INCREMENT. Bovendien geeft Jan de GET-nummers zelf mee én voert hij er al een controle op uit:
Als ik via GET nummers meegeef en ik doe natuurlijk ook controle of het wel nummers zijn […]
Kortom, als je al een controle uitvoert op de geldigheid van nummers die je nota bene zelf genereert, dan cast (int) eventueel ongeldige invoer naar niets meer dan een 0 die bij een AUTO_INCREMENT ook nog ongeldig is.
Nee, na een controle op nummers is escapen helemaal nergens voor nodig …
Maar zoals ik hier boven in mijn relaas aangeef spelen er ook andere overwegingen - je wilt een consistente werkwijze waarbij je alle user data op eenzelfde wijze behandelt zodat je je niet bij elk DATA-deel in je SQL wilt afvragen "is dit nu expres niet ge-escaped omdat dit niet nodig is of ben ik dit hier toch vergeten?". Dit werkt gewoon niet prettig en nog belangrijker, dit is erg foutgevoelig. Dus, pas overal dezelfde werkwijze toe, of gebruik prepared statements zoals @Rob aangeeft, die dwingt dit, mits correct toegepast, voor je af.
Waarom niet ctype_digit?
Accepteert meer invoer dan waarin je geïnteresseerd bent. Als je een auto-increment id verwacht, dan zou ik een controle uitvoeren die daarop is toegespitst en enkel een (syntactisch) geldig auto-increment id accepteert. Dit is dan je syntactische check, uiteraard moet je dan ook nog een semantische check uitvoeren waarin je controleert of het id ook daadwerkelijk refereert aan een bestaand record.
@Ward heeft in die zin gelijk dat het geen "risico's" oplevert ten aanzien van de veiligheid van de query, en ook levert zo'n typecast een syntactisch correcte query op, maar tegelijkertijd houdt zo'n typecast mogelijk ook in dat je potentieel verkeerde invoer aanpast/"repareert" (wat mij onwenselijk lijkt) en de query waarin het cijfer 0 dan wordt opgenomen zal ook nooit het gewenste resultaat opleveren. De vraag is dan of als de query geen resultaat oplevert of dan de verdere verwerking van data wordt stopgezet/teruggedraaid zodat er geen rotzooi in de database komt.
Het lijkt mij gewoon makkelijker om alles aan de poort te controleren en als dit in eerste instantie al niet (exact) voldoet dat je dan alle vervolgacties (zoals queries die dan toch geen zinnig resultaat meer opleveren) gewoon afblaast.
Ik snap wat je bedoelt hoor Ward, maar het is geen controle in die zin dat je controleert of de input klopt. Dat is wat ik bedoelde te zeggen.
Stel je hebt vierkantjes nodig. Wat je doet bij een controle is kijken of alles wat binnenkomt een vierkant is. Zo niet, dan staak je de huidige actie. Bij typecasting zeg je, maakt niet uit wat er allemaal binnenkomt (rondjes, driehoeken enz.) ik maak overal een vierkantje van en ga vrolijk verder. Dat kan, maar het is iets anders dan een controle.
Jan heeft al een controle op de nummers. Als je niet 100% zeker weet dat die controle helemaal waterdicht is, kun je ná die controle nog een typecast naar een integer toepassen.
Ik beantwoorde daarmee de concrete vraag wat directer (ga je met die stappen SQL-injectie tegen), maar we zijn het verder vooral erg eens. ;-)
Zelf gebruik ik voor dit soort ID's overigens vaak data objects juist omdat je daarmee méér kunt formaliseren dan alleen een "dit is een integer". Bij een AUTO_INCREMENT mag zo'n data object bijvoorbeeld niet kleiner dan 1 zijn, iets wat bij een gewone integer wel mag. Voldoet input niet aan de norm, dan rolt er al bij het genereren van het data object een exception uit.
Desalniettemin is het uitvoeren van een query met een syntactisch juiste, maar semantisch onzinnige, waarde... onzinnig. Zorg gewoon dat de validatie afgerond is voordat je uberhaupt aan queries begint, dingen on the fly fixen/patchen lijkt mij niet ideaal.
Als je niet 100% zeker weet dat die controle helemaal waterdicht is, kun je ná die controle nog een typecast naar een integer toepassen.
Of je zorgt ervoor dat de controle de lading wel dekt of dat het opnemen van DATA in SQL altijd veilig is (of nog beter, beide)? :/
Ja, je gaat met een typecast wellicht SQL-injectie tegen, maar dit is zoiets als je tuin in de fik steken met kerosine, omdat dit een insektenplaag kan voorkomen. Het neemt dan weliswaar een ongewenst effect weg, maar introduceert weer andere ongewenste dingen.
Dan moet je je afvragen hoe je je andere DATA-delen beschermt in je query, gebruik je hiervoor andere methoden/truuks? Zou het niet veel makkelijk zijn om hier een (één) lijn in aan te brengen? Zodat je - als je over een tijd je code nog eens overleest of raadpleegt, je niet elk element moet inspecteren om te zien wat hier aan de hand is? Al die elementen zullen dan stuk voor stuk op een juiste (specifieke) aanpak geïnterpreteerd moeten worden. Elke keer opnieuw. Idealiter wil je een makkelijke manier die veilig is, en dat is echt niet moeilijk om voor elkaar te krijgen.
En nogmaals, het controleren/repareren van invoer en het op een veilige manier uitvoeren van queries zijn twee verschillende dingen. Behandel deze dan ook in afzondering (separation of concerns).
