MySQLi prepared statements

TL;DR

• Een methode is van zichzelf nooit veilig, er is enkel een veilig gebruik van een methode
• Prepared statements in MySQLi zijn bagger, als je dan toch prepared statements moet gebruiken, doe dit dan via PDO
• vertrouw nooit user input
• filter input, escape output

Een werkwijze is van zichzelf niet zomaar veilig (alsof je "abracadabra!" roept, en je dan beschermd bent :)), enkel als je een werkwijze juist toepast heb je wat meer garantie dat iets veilig is.
Bovenstaande code is misschien veilig (maar dit is niet zeker, waarover later meer), maar dat is geen constructie met prepared statements - je bent daar simpelweg een querystring aan het opbouwen.

Daarnaast is het gebruiken van prepared statements "omdat dit veilig zou zijn" misschien niet helemaal de juiste insteek en lang niet de énige reden/overweging waarom je dan maar altijd prepared statements zou moeten/mogen gebruiken.

Bij elke werkwijze komt ook een zekere discipline kijken die ervoor zorgt dat je de werkwijze op de juiste manier toepast. Je kunt niet voor prepared statements kiezen, maar in je stoel achterover leunen en een partij baggercode schrijven want die is mogelijk even (on)veilig als een andere gebruikte methode geschreven in dezelfde "mindset".

Ook MOET je begrijpen wat prepared statements functioneel doen en wat het gebruik hiervan impliceert.

Los van de voorziening van MySQLi in PHP, die een handjevol functies heeft om met prepared statements te werken, is dit een voorziening van MySQL zelf. Dit is geen laag die enkel in PHP bestaat maar ook een laag in MySQL zelf. Prent dit goed in je gedachten.

Dit in tegenstelling tot PDO waarin je alleen maar kunt werken met prepared statements, maar niet noodzakelijkerwijs enkel met MySQL (maar met een andere database type die mogelijk niet native prepared statements ondersteunt).

In PDO heb je een optie genaamd PDO::ATTR_EMULATE_PREPARES. Deze (boolse) waarde geeft aan of prepared statements ge-emuleerd moet worden (bijvoorbeeld omdat het database-type deze voorziening niet heeft) of dat er van de native prepared statement functionaliteit van het database-type zelf gebruik gemaakt dient te worden. Als je deze waarde dus op false zet (dat wil zeggen, de emulatie staat UIT), communiceert PDO op dezelfde wijze met MySQL als dat je via MySQLi+prepared statements zou doen.

Wat houdt dit in? Indien je gebruik maakt van prepared statements in de "native MySQL sense" dan communiceer je via een compleet ander protocol met MySQL dan wanneer je niet van prepared statements gebruik zou maken. Het aantal queries (vraagstukken) dat je de database stelt is ook compleet anders!

Wat er "in a nutshell" gebeurt is het volgende: elke keer als jij een prepared statement uitvoert stuur je een SQL-sjabloon (het prepared statement) naar je database (1 query). Vervolgens ken je waarden of variabelen (in MySQLi kun je enkel variabelen binden geloof ik, niet simpelweg waarden, het kan wel allebei in PDO) toe aan de placeholders van dit sjabloon en voer je het prepared statement uit met execute (dit is NOG 1 QUERY). Elke "query" die je wilt uitvoeren resulteert -in MySQLi icm prepared statements in ieder geval- in het sturen van TWEE queries naar je database!

Dan spelen er een aantal overwegingen: wanneer is dit zinnig? En we redeneren hier dan even vanuit efficiëntie, niet vanuit security.

Bij een (ELK!) SELECT statement? 2 queries. Meh.

Bij een INSERT statement? Misschien, als je meerdere inserts uitvoert? Maar bij één INSERT, waar meestal per keer sprake van is? Meh.
EDIT: dit is een argument voor het gebruik van prepared statements: als je veelvuldig het sjabloon (prepared statement) hergebruikt met verschillende parameter-waarden dan zijn prepared statements mogelijk (veel) sneller: bij elke query -na de eerste- hoeft er enkel data van de parameters over de lijn. De query zelf is al geparsed en correct bevonden et cetera. Aan de MySQL kant staat alles al in de startblokken om de query (het querysjabloon (herhaaldelijk)) verder te verwerken.

Bij een UPDATE of DELETE? Zelfde als bij een INSERT.

Daarnaast is het gebruik van bind_param() compleet pet en ziet er nogal "clunky" uit. Een geserialiseerde string met letters om typehints van de te binden variabelen mee te geven? WTF?

Nee, nee, driewerf nee.

Kijk, als je dan toch prepared statements MOET gebruiken (zonder opgaaf van reden zou ik dit niet eens accepteren), gebruik dan PDO met simulatie aan (PDO::ATTR_EMULATE_PREPARES op true).

Okay, dan terug naar jouw code. Ik denk dat je even een stap terug moet nemen en een zekere denkwijze moet gaan volgen.

Stel je het volgende voor: je werkt met een HTML document. Hierin kunnen andere mensen berichten plaatsen (denk aan een gastenboek). Je wilt natuurlijk niet dat er allerlei onzin in gezet kan worden (HTML die je layout breekt, JavaScript die je cookies steelt). Dus je wilt de invoer op een of andere manier onschadelijk maken, in dit geval in de HTML context. Dit concept is belangrijk: je werkt in een bepaalde context en je wilt DATA, die afkomstig is uit een of andere externe bron, ontdoen van de mogelijk speciale betekenis binnen die context. Hiervoor gebruik je bijvoorbeeld htmlspecialchars(). Klus geklaard, alle flauwekul in je gastenboek geneutraliseerd.

Zo geldt dit ook voor andere contexten: in de SQL-context heb je een real_escape_string() functie die hetzelfde doet (MITS je quotes om deze DATA zet, het een (real_escape_string()) zonder het ander (quotes) is NIET VEILIG). Ook hier is wederom belangrijk: REAL_ESCAPE_STRING() IS GEEN WONDERMIDDEL!

Maar het is daarbij handig om ALLE DATA op eenzelfde manier te behandelen. Al die DATA is in zekere zin afkomstig uit een externe bron, die je niet zou moeten vertrouwen. Het is ook veel makkelijker in het gebruik, want je hoeft je dan niet af te vragen of je de DATA kunt vertrouwen (of er al ooit een controle is uitgevoerd) of niet. Ga er gewoon altijd vanuit dat de DATA niet betrouwbaar is. $gebruikersID zou dus in je query dezelfde behandeling moeten krijgen als $sitenaam. Deze zou je dus ook moeten escapen.

En als je slim bent schrijf je een wrapper voor je MySQLi functies/methoden. Waarbij je bij voorkeur object georiënteerd werkt. Desgewenst heb ik hier wel code voor.

Maar terugkomend op de eerdere escaping, onthoud de volgende vuistregel: filter input, escape output.

Om ten langen leste antwoord te geven op je vraag: jouw bovenstaande code is niet per definitie veilig, omdat je niet al je DATA escaped, ook al komt deze uit je sessie en zou deze al veilig moeten zijn. Maar daarbij doe je een aanname. En je wilt niet, dat als deze ketting van aannames wordt doorbroken, je site ineens niet meer werkt of wel?

Citaat van matistop333

Nu heeft een leraar mij gezegd dat ik prepared statements moet gebruiken tegen SQL Injections.

Deze uitleg vind ik trouwens een leraar onwaardig. De docent zou beter moeten uitleggen hoe je prepared statements veilig toepast, en hoe dat bijdraagt in het voorkomen van SQL injection. Dan zal waarschijnlijk blijken dat prepared statements een groot deel van de eerder genoemde escaping automatisch voor hun rekening nemen. Maar dat wil niet zeggen dat, simpelweg omdat je dan een truukje volgt waarvan je niet precies begrijpt hoe deze werkt, je dit maar blind moet volgen en dan altijd "veilig"bent... Dat is het onheil over je afroepen.

Indien je een wrappertje schrijft voor de normale werkwijze in MySQLi (zonder prepared statements) zou dit er bijvoorbeeld als volgt uit kunnen zien:

<?php
$db->query(
    "INSERT INTO Websites (Naam, GebruikersID) VALUES (
        '".$db->escape($_POST['sitenaam'])."',
        '".$db->escape($_SESSION['gebruikersID'])."'
    )"
);
?>

En om je een voorbeeld te geven van real_escape_string() is geen wondermiddel:

Gegeven het volgende codefragment:

<?php
// Connect etc.
// ...


$id = isset($_GET['id']) ? $_GET['id'] : false;


if ($id === false) {
    ?><p>No id supplied.</p><?php
} else {
    $res = mysqli_query(
        $link,
        'SELECT * FROM table WHERE id = '.mysqli_real_escape_string($link, $id)
    );
    // etc.
}
?>

Is deze query veilig?

Het antwoord is NEEN.

mysql_real_escape_string() escaped alleen maar indien er iets te escapen valt.

Als je voor je id het volgende invult:

41 OR 1=1

Dan doet real_escape_string() hier NIETS mee. Hiermee kun je dus een geslaagde SQL injectie uitvoeren.

Dit is de reden waarom je real_escape_string() ALTIJD zou moeten gebruiken in combinatie met quotes. Het een is simpelweg niet veilig zonder het ander.

Nu heb ik hier lang en breed over na zitten denken (en ben er eigenlijk nog steeds niet helemaal uit). Stel nu dat je je ook bedient van input filtering (de andere helft van filter input, escape output). Wat nu als je er eerst voor zorgt dat je alleen de query uitvoert indien $_GET['id'] een numerieke waarde heeft? Je zou dan escaping geheel achterwege kunnen laten (al zou ik dat waarschijnlijk niet aanbevelen :)). Voor beide aanpakken of een combinatie hiervan valt iets te zeggen. Zaak is en blijft dat je altijd code schrijft waarbij je aanneemt dat ALLE externe DATA een potentieel risico vormt en deze ook als zodanig behandelt.

Uhm, gewoon altijd real_escape_string() in combinatie met single quotes in je querystring.

WEL veilig:

<?php
$res = mysqli_query(
    $dbLink,
    "SELECT whatever FROM table WHERE id = '".mysqli_real_escape_string($dbLink, $someVar)."'"
);
?>

NIET veilig:

<?php
$res = mysqli_query(
    $dbLink,
    'SELECT whatever FROM table WHERE id = '.mysqli_real_escape_string($dbLink, $someVar)
);
?>

Nog beter zou een OOP variant van het eerste fragment zijn, uiteraard.