[PHP] Risico SQL injectie

[mosymuis]

Ik ken het verhaal van de slashes op GET/POST/COOKIE variabelen. Door de gebruiker ingevoerde waardes mogen nooit direct in een query geplaatst worden zonder geslashte quotes. Maar wat als men nu een script als dit gebruikt, waar magic_quotes_gpc aan staat?

code:

$sql = "INSERT INTO tabel
  SET veld = '".$_POST['waarde']."';";
mysql_query($sql);

Hier werden de quotes al geplaatst door PHP, en dus staan ze nog steeds in de query. Maar wat is dan de meerwaarde van de functie mysql_real_escape_string(), wat veiligheid betreft? Deze gebruik ik altijd; ik haal $_POST vars binnen met onder andere stripslashes() en plaats ze daarna met deze functie in de $sql opdracht.

Is deze aanpak werkelijk veiliger voor SQL injectie dan het code fragment hierboven, of is het overbodige moeite? Kunnen NULL-bytes, tabs, newlines etc. last veroorzaken wanneer men HTTP requests tweakt?

Het gaat me hier nu alleen even om dit risico; ik begrijp dat je user input principieel zou moeten controleren op inhoud, waar mogelijk. Ik heb het hier ook niet over de weergave; ingevoerde HTML code filter ik altijd pas achteraf.

[ShadowLord]

Je vraag is niet helemaal duidelijk, maar als ik het goed lees komt het neer op: magic_quotes_gpc gebruiken of mysql_real_escape_string()?

Het probleem met magic_quotes_gpc is simpel... je kan er niet op rekenen dat deze functie altijd aanstaat. En code om dit te checken kun je wel schirjven, maar je zal zien dat je dit ergens een keer vergeet en dan heb je de spreekwoordelijke stront aan de knikker.

Daarom is het beter om uit gewoonte (wat jij dus al doet) ALTIJD je strings te escapen. Op die manier kun je minder fout gaan.

OK, ik hoor nu de vraag: "Je moet er altijd code bij zetten, dus wat maakt het uit?" Nou, het checken van magic_quotes_gpc doe je waarschijnlijk maar 1x aan het begin van je code. Staat ie uit, zet je hem aan (want je bent gewent zo te coden, anders heeft het geen voordeel - nog steeds iedere query een string escape nodig indien het uit staat). Nu zal het dus een keer gebeuren dat je die code aan het begin vaan een pagina vergeet, of de code wordt om een of andere reden niet uitgevoerd.
Het altijd escapen is veel makkelijker te checken aangezien het sneller opvalt als er een mist bij een query.

[mosymuis]

Mijn vraag is, wat de meerwaarde van mysql_real_escape_string() is boven het normaal escapen (door magic_quotes, dan wel addslashes). Ik zie niet hoe de extra tekens die deze functie aanpakt, de query kunnen beïnvloeden?

[bartvb]

Het antwoord op die vraag staat gewoon in de PHP manual
(korte antwoord: ja, real_escape gebruiken is veiliger)

[ACM]

mosymuis schreef op donderdag 08 september 2005 @ 11:18:
Mijn vraag is, wat de meerwaarde van mysql_real_escape_string() is boven het normaal escapen (door magic_quotes, dan wel addslashes). Ik zie niet hoe de extra tekens die deze functie aanpakt, de query kunnen beïnvloeden?

Addslashes is een soort half-bakken default vanwege mysql. Vroeger kon je daar de ' alleen met een \ ervoor escapen en moest je de \ dan ook escapen.
Het huidige nut van de functie ontgaat me een beetje, want bij de meeste SQL-implementaties (ook mysql kan dat) hoef je alleen de ' te escapen met een 2e ' ('' dus) en verder niets. Met name het escapen van de " is bijzonder onzinnig in het licht van databases.

Ik geloof niet dat er uberhaupt een stuk software is dat echt wat aan addslashes heeft. Voor je database kan je beter elke waarde die je erin mikt controleren op validiteit (getallen enzo) en de overgebleven "gevaarlijke dingen" (varchars en texts dus) met de database-specifieke regels escapen.

Voor regexpen moet je de waarden toch met quote_meta escapen omdat je niet weet welke meta-chars allemaal gebruikt kunnen worden of vanwege je delimiters een bijzondere waarde hebben.
En voor output naar bijvoorbeeld javascript voldoet de addslashes-methode vziw ook niet helemaal. Zo mag de ' meen ik niet escaped worden in een "-omsloten string en vice versa.

[BierPul]

PHP:

function doQuote($str) {
    if(get_magic_quotes_gpc() == 0) {
        $str = mysql_real_escape_string($str);
    }
    return $str;
}

Ik gebruik altijd zo'n soort functie waarmee ik mn strings valideer voordat ze de query in gaan.
De integers cast ik altijd voor de zekerheid nogmaals naar (int). Zo kan je volgens mij weing gebeuren

[ACM]

Die functie houdt dus niet echt rekening met al waarden die niet uit GPC komen, of wel?

Dat is nog een nadeel van die magic_quotes, je moet bij gaan houden waar een variabele vandaan kwam... Kwam ie uit gpc, dan hoef je hem niet meer te escapen, kwam ie uit je app of de db, dan moet ie dat wel. Dus imho voegt magic_quotes meer ellende toe dan dat het wegneemt.

[Verwijderd]

ACM schreef op donderdag 08 september 2005 @ 13:27:
Die functie houdt dus niet echt rekening met al waarden die niet uit GPC komen, of wel?

Dat is nog een nadeel van die magic_quotes, je moet bij gaan houden waar een variabele vandaan kwam... Kwam ie uit gpc, dan hoef je hem niet meer te escapen, kwam ie uit je app of de db, dan moet ie dat wel. Dus imho voegt magic_quotes meer ellende toe dan dat het wegneemt.

Dat klopt. Ik zie het als good practice om te zien of magic_quotes "aan" staan, en als dat het geval is, het script gewoon helemaal af te kappen. Laat de beheerder maar zorgen dat de instellingen juist zijn (desnoods alleen voor het betreffende script/de directory/de virtual host).

[BierPul]

Goed punt

Ben er alleen nog niet echt tegen aan gelopen, misschien omdat je op het moment dat het uit de eigen applicatie komt al weet wat je kan verwachten en het daarom netjes afhandeld.

Of misschien teveel vertrouwen op mezelf

Maar alsnog vraag ik me op dit moment het echte gevaar af, ik heb wel wat research gedaan maar verder dan een SQL error afdwingen kwam ik eigenlijk niet.

[bakkerl]

of pear::DB gebruikt

PHP:

$sql = "SELECT * FROM tabel WHERE stringval=? AND intval=? AND postval=?";
$sqldata = array("value1", 42, $_POST["waarde"]);
$result = $db->query($sql, $sqldata);

$db is hierbij een openstaande verbinding naar een database.

Alle mogelijke sql dingen worden eruit gehaalt. Netjes '' om strings heen, waardes escaped waar nodig....

[ Voor 10% gewijzigd door bakkerl op 08-09-2005 13:56 . Reden: Uitbreiding met postval ]