Door
John De Zon
op 04-09-2015 14:53
gewijzigd op 05-09-2015 18:38
4.476 views
Hallo,
ik ben bezig met een registratie script voor m'n website. Ik heb me voorgenomen de beveiliging zo goed als ik kan te maken vanaf het begin.
Is dit script veilig? En waarom geeft mysql_real_escape_string niets terug?
Trimmen is niet slim op een wachtwoord veld. Het is een extra karakter die mensen kunnen gebruiken in hun wachtwoord.
Ik zeg nergens dat dat de enige controle zou moeten zijn of dat je dit altijd en overal blind zou moeten toepassen. Je moet per veld bepalen wat nodig is. Als een veld verplicht is, is het controleren of deze "leeg" is een minimale controle. Ik zeg dat je de getrimde variant moet vergelijken met een lege string, NIET dat je de invoer moet AANPASSEN naar de getrimde variant.
Johan K op 04/09/2015 18:48:46
Een simpele regex is voldoende
Hoe is een regex geen whitelist?
Johan K op 04/09/2015 18:48:46
Als er een classe word gemaakt zou het "escapen" eigenlijk automatisch moeten gaan.
Nee, niet automatisch nee. Tenzij je een prepared statements laag aan het bouwen bent, in welk geval je beter van PDO gebruik kunt maken (die van MySQLi is brak). Naar mijn mening moet je dit soort dingen niet automatiseren omdat er dan geen bewustzijn meer is.
Daarnaast wil je soms nog steeds de keuze hebben om op bepaalde plaatsen rauwe data te gebruiken, je wilt dan niet hebben dat deze data automatisch ge-escaped wordt. Je wilt niet dat je klasse je op voorhand beperkt in je handelen.
@Aar, je wilt de hele string controleren dit doe je met ^...expressie...$. Daarnaast heb je de de /i modifier die een case-insensitive match doet. Hierbij moet je ook niet vergeten dat $ ook één newline karakter (\n) accepteert wat vaak vergeten wordt en dat de return-waarde van preg_match gelijk moet zijn aan 1.
Een match voor een (niet-lege) string die enkel uit alfanumerieke karakters zou bestaan wordt dus zoiets:
<?php
$input = 'lala123';
if (preg_match('#^[a-z0-9]+$#i', $input) == 1) {
echo 'input bestaat enkel uit alfanumerieke karakters';
// en voor de goede orde zou je $input hier nog moeten trimmen!
} else {
echo 'input bestaat NIET enkel uit alfanumerieke karakters of is leeg';
}
?>
Zelf ook ff testen natuurlijk...
Met accolades zou je nog een minimale/maximale lengte aan kunnen geven.
{exacte_lengte}
{minimale_lengte,}
{mininmale_lengte, maximale_lengte}
Ik zeg nergens dat dat de enige controle zou moeten zijn of dat je dit altijd en overal blind zou moeten toepassen. Je moet per veld bepalen wat nodig is. Als een veld verplicht is, is het controleren of deze "leeg" is een minimale controle. Ik zeg dat je de getrimde variant moet vergelijken met een lege string, NIET dat je de invoer moet AANPASSEN naar de getrimde variant.
Ik vergelijk niets met een lege string, hou het gewoon lekker simpel en voer direct de regex uit op velden waar nodig is zoals een username & password. Op andere velden zoals checkbox of optionele velden voldoet empty().
Thomas van den Heuvel op 04/09/2015 19:45:32
Hoe is een regex geen whitelist?
Is het ook, maar je maakt het moeilijker dan dat het is.
Thomas van den Heuvel op 04/09/2015 19:45:32
Nee, niet automatisch nee. Tenzij je een prepared statements laag aan het bouwen bent, in welk geval je beter van PDO gebruik kunt maken (die van MySQLi is brak). Naar mijn mening moet je dit soort dingen niet automatiseren omdat er dan geen bewustzijn meer is.
Daarnaast wil je soms nog steeds de keuze hebben om op bepaalde plaatsen rauwe data te gebruiken, je wilt dan niet hebben dat deze data automatisch ge-escaped wordt. Je wilt niet dat je klasse je op voorhand beperkt in je handelen.
PDO bindParam / bindValue escaped alles als string tenzij anders is doorgegeven of je wilt je zelf open stellen om user input direct in je sql te gebruiken.
Ik vergelijk niets met een lege string, hou het gewoon lekker simpel en voer direct de regex uit op velden waar nodig is zoals een username & password. Op andere velden zoals checkbox of optionele velden voldoet empty().
Ik heb het ook niet over wat jij doet, ik voelde mij genoodzaakt op opnieuw/beter toe te lichten wat ik bedoelde, omdat het antwoord wat jij gaf mij deed denken dat ik invoer aan het aanpassen (trimmen) was...
Johan K op 05/09/2015 11:18:56
Is het ook, maar je maakt het moeilijker dan dat het is.
Daarom zeg ik ook dat je per veld moet bepalen wat nodig is...
Johan K op 05/09/2015 11:18:56
PDO bindParam / bindValue escaped alles als string tenzij anders is doorgegeven
Oh really, zet querylogging eens aan en kijk wat PDO daadwerkelijk doet. 9 van de 10 keer negeert deze gewoon typehints en zet om alle DATA quotes heen (in de uiteindelijke query). But hey, don't take my word for it.
Johan K op 05/09/2015 11:18:56
of je wilt je zelf open stellen om user input direct in je sql te gebruiken.
Dat lijkt mij duidelijk niet de bedoeling. Maar goed, het kan zijn dat jij nooit rauwe SQL passages nodig hebt gehad in je query en hier dus nog nooit tegenaan bent gelopen.
Ik heb het script geüpdatet. Ik denk dat ik zowat alle tips heb gebruikt...
Dat van die whitelist ga ik niet meer doen omdat ik het nut er niet meer kan inzien. Dank voor het uitleggen van de Whitelist ik zal dit zeker meenemen.
Is het script veilig? Zijn er eventueel nog dingen die beter moeten?
Even tussendoor: weet je hoe vervelend het is als je een wachtwoord intypt op je telefoon en dat die dan automatisch een spatie erachter zet en daardoor dus niet geldig is ?
Een rtrim($iets) zou echt handig zijn.
Ook bij emailadressen: mijn emailadres staat in mijn woordenboek (Swiftkey), dus als ik maar 4 letters type, dan vult hij het aan. Met een spatie. Nu moet ik die altijd verwijderen (heel vervelend) omdat het anders verkeerd kan gaan.
Trouwens: facebook bijvoorbeeld niet. Als je daar je emailadres invult bij het inloggen MET een spatie, dan haalt ie die spatie weg.
Waarschijnlijk kijkt hij eerst of je rauwe input bestaat. Zo niet: is een rtrim() anders dan de rauwe input? Zo ja: probeer het eens met rtrim() erover..