Ben ik beschermd tegen SQL injection?

Door Michael Desmadril op 15-12-2014 22:04 gewijzigd op 15-12-2014 22:20

8.580 views

Goeieavond,

Ik ben nu een 2-tal weken bezig met php & mysql. Ik werk aan de hand van een boek die pas uitgekomen is en van de recentste Lynda.com videos. Ik focus me op Mysqli ipv PDO omdat ik zelf toch uitsluitend werk met Mysql. Het bevalt me enorm! Het enige dat er natuurlijk gebeurd is dat je na een tijdje begint te experimenteren. Het werkt en je bent blij...maar is het wel goed?...of correct?

Ik heb bijvoorbeeld iets simpels in elkaar geflanst, vlug vlug...

Doordat men enorm waarschuwt voor sql injection werk ik met prepared statements....

Kan iemand mij vertellen of mijn spaghetticode op "iets" trekt en veilig is? Ben ik op de goede weg?


<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="utf-8">
    <title>title</title>
    <link rel="stylesheet" href="style.css">
    <script src="script.js"></script>
  </head>
  <body>
  	<form action="homes.php" method="POST">
  		<p>Zoek: <input type="text" name="search" /></p>
  		<p>Voornaam: <input type="text" name="Voornaam" /></p>
  		<p>Achternaam: <input type="text" name="Achternaam" /></p>
  		<p>Adres: <input type="text" name="Adres" /></p>
  		<p>Discipline: <input type="text" name="Discipline" value="1 tot 5" /></p>
  		<p>Bevestig zoeken: <input type="submit" name="Submit" /></p>
  		<p>Voeg toe aan databank <input type="submit" name="Add_DB" value="Add to Database" /></p>
  	</form>
  <?php
	require_once 'isset.php';
	?>
  </body>
</html>



This is the PHP

<?php
	require_once 'login.php';
	$dbcon =  mysqli_connect($db_host, $db_username, $db_password, $db_database);
	if(mysqli_connect_errno()) die ("Error during connection");

	if(isset($_POST['Submit'])){
		

		$Naam = $_POST['search'];

		$Result = mysqli_query($dbcon,"SELECT * FROM customers WHERE Voornaam = '$Naam'");
		if(!$Result) die ("Nothing to show");

		$Rows = $Result->num_rows;

		for($i=0; $i < $Rows; $i++){
			$Row = mysqli_fetch_array($Result, MYSQLI_ASSOC);
			echo "Voornaam: " . $Row['Voornaam'] . "<br>";
			echo "Achternaam: " . $Row['Achternaam'] . "<br>";
			echo "Adres: " . $Row['Adres'] . "<br>";
			echo "<hr>";
			
		}
	}

		if(isset($_POST['Add_DB'])){
			$sql="INSERT INTO customers(Voornaam, Achternaam, Adres, Actief, Discipline)
				VALUES(?,?,?,NOW(),?)";

			if($stmt = $dbcon->prepare($sql)){

					$stmt->bind_param('sssi', $Voornaam, $Achternaam, $Adres, $Discipline);
					
					$Voornaam = $_POST['Voornaam'];
					$Achternaam = $_POST['Voornaam'];
					$Adres = $_POST['Adres'];
					$Discipline = $_POST['Discipline'];
					$stmt->execute();
					

					echo "New records created successfully";
}
		}

		mysqli_close($dbcon);
?>

Groeten,
Michaël

Onbekende gebruiker

16-05-2015 23:41

SQL Injection is wat het aangeeft: dat iemand zonder het weten of te mogen data kan aanleveren die door de database wordt herkend als SQL. Ofwel dat het niet duidelijk is voor de database welk deel nu de SQL code is, en welk deel de data. Het grootste probleem is dat je als PHP programmeur soms zoveel met queries werkt dat het een klus kan zijn om alle queries adequaat te beschermen.
Meestal wordt aangeraden om mysqli_real_escape_string() te gebruiken. Maar dat werkt niet altijd, bijvoorbeeld als je een ID wilt escapen en iemand gebruikt '1 OR 1=1'. Dan moet je in ieder geval nog alle data checken op type, dat is al een hele klus.
Daarom is de enige aanbevolen manier om met prepared statements te werken. Omdat het aanbieden van de query en de data in gescheiden stappen gaat, is er voor de database geen misverstand wat precies de query is en wat de data is.

Paco de Wulp

17-05-2015 14:04 gewijzigd op 17-05-2015 14:13

Laatste tijden veel aan mijn PHP-scrips verbeterd.
1) Quote's regeltjes consequent overal toegepast (wanneer enkele quotes en wanneer dubbele quotes)
2) Overgestapt van PHP 5.3.29 naar PHP 5.5.24
3) MySQL-statements omgezet naar MySQLi
4) SQL-injection d.m.v. mysqli_real_escape_string afgevangen
5) Password encryptie toegepast (password_hash, password_verify, password_needs_rehash)
6) Javascripts verbeterd (o.a. IBAN check toegevoegd)

Nu zit ik nog met 2 puntjes:

1) Ik wil 'prepared statements' gaan toepassen om het nog veiliger te maken.
Mijn vraag is: Is het zinvol om gewoon alle SQL-statements om te zetten naar een versie van prepared statements ? Kan ik dus al mijn mysqli_real_escape_string-statements er weer uitgooien ?

2) Ik zit nog met een vraag (waarschijnlijk een domme vraag).
Ik heb middels javascript alle velden gecontroleerd (of het een juist IBAN-nummer is, of het veld niet leeg is, of de datum-va/dataum-tm goed zijn ingevuld, etc...). Moet ik nu alle javascript-controles ook nog eens bij de PHP-validaties doen? Want ik begrijp dat je javascript uit kan zetten en dan worden er geen javascript-validaties gedaan. Maar is dit niet allemaal een beetje dubbel op ??

[size=xsmall]Toevoeging op 17/05/2015 16:05:18:[/size]

3de puntje:
AJAX maakt gebruik van javascript. Wanneer javascript wordt uit gezet in de browser werkt AJAX dan ook niet meer ?

Ward van der Put

17-05-2015 16:22

>> 1) Ik wil 'prepared statements' gaan toepassen om het nog veiliger te maken.
Mijn vraag is: Is het zinvol om gewoon alle SQL-statements om te zetten naar een versie van prepared statements ?

Nee, het gaat alleen om de injectie van buitenaf. SQL-expressies die geen output van buitenaf bevatten, kunnen niet met SQL-injectie worden misbruikt.

>> Ik heb middels javascript alle velden gecontroleerd (of het een juist IBAN-nummer is, of het veld niet leeg is, of de datum-va/dataum-tm goed zijn ingevuld, etc...). Moet ik nu alle javascript-controles ook nog eens bij de PHP-validaties doen?

Ja. Niet alleen kan JavaScript worden uitgezet: een hacker kan elk HTTP-request veranderen. Alle input die van buiten komt is dus bij voorbaat verdacht, totdat je met controles hebt bewezen dat deze ermee door kan.

Paco de Wulp

17-05-2015 19:07

Dank je @Ward.
ad.1)Maar wat is erop tegen om alles met prepared-statements doen? Het kan toch geen kwaad. Je hanteert dan wel tenminste overal dezelfde methodiek. Of heeft het andere consequenties voor bijvoorbeeld de performance, of is het niet sjiek om iets-wat overbodige statements in je code te hebben ?

ad.2)Dus eigenlijk kan je alle javascript checks skippen, want alles moet toch in PHP worden gecheckt. Eigenlijk zijn javascripts dus alleen maar voor een iets mooiere schermafhandeling, qua foutmeldingen en ander soort meldingen. Toch ?

ad.3) Maar hoe zit het dan met AJAX die ook gebruik maakt van javascript ?

Frank Nietbelangrijk

17-05-2015 19:26 gewijzigd op 17-05-2015 19:57

1) Er is niets op tegen om alles met prepared statements te doen. Al zou het iets trager zijn dan merk je daar helemaal niets van. Maar Ward heeft gelijk. Vaste queries zonder variabelen van buitenaf zijn voor een buitenstaander ook niet te misbruiken.

2) Alles moet met PHP gevalideerd worden. Het is echter vaak wel gebruiksvriendelijk om ook javascript validatie te gebruiken. Denk bijv. aan een vakje dat direct rood wordt nadat de focus van het veld af is.
De gebruiker kan nu direct het vakje alsnog juist invullen en hoeft later niet nog een keer terug.
Persoonlijk vind ik dat met de komst van HTML5 de javascript validatie grotendeels weggelaten kan worden. Nu heb je immers de mogelijkheid om een input type "email" en "numeric" e.d. te maken.

3)Ook bij AJAX moet je serverside valideren en kun je om het gebruiksvriendelijker te maken ook clientside valideren.

Thomas van den Heuvel

17-05-2015 20:02

Paco de Wulp op 17/05/2015 14:04:46
4) SQL-injection d.m.v. mysqli_real_escape_string afgevangen

Mijn god daar gaan we weer.

mysqli_real_escape_string is geen wondermiddel.

Stel dat jij een query hebt waar je voor een zekere parameter een getal verwacht, zet je daar dan quotes omheen?

Bijvoorbeeld: SELECT * FROM users WHERE user_id = $x.

$x komt bijvoorbeeld uit $_GET['id'].

Als je geen quotes gebruikt doet je real_escape_string niets. Als $_GET['id'] "OR 1 = 1" bevat (zonder dubbele quotes) dan wordt er niets geescaped omdat er niets te escapen valt.

Als je wel quotes gebruikt... ik weet niet, het is wel "veilig" in combinatie met mysqli_real_escape_string() maar het is nogal onzinnig.

Als je prepared statement gebruikt... doe je een hoop werk voor niets als je het mij vraagt. prepare(), bind_param(), etc. Aint nobody got time fo dat.

Bottom line: als je een getal verwacht controleer dan op een getal. Oftewel filter je input. Als je invoer niet voldoet aan een zeker formaat dan hoef je ook geen query uit te voeren die niets oplevert.

Paco de Wulp

17-05-2015 20:19

Dank je @Frank. Duidelijk.
@Thomas, mij hoor je er niet meer over...ik snap het nu. Be Happy !!

Thomas van den Heuvel

17-05-2015 20:32

Ik zie nergens filter input, escape output staan dus ik behoud mij het recht voor enige reserveringen te hebben (en te houden) ten aanzien van bovenstaande stelling.

Je "regels" zijn ook voornamelijk gericht op het buiten houden van rotzooi, maar over een verdere behandeling van "user data" hoor ik je niet. Tenzij jij bepaalde gebruikers in je systeem/systemen vertrouwt (middels een soort van chain-of-trust waarbij privileges worden doorgegeven) moet je alle user data blijven behandelen als zijnde onbetrouwbaar, zelfs (en wellicht vooral) als deze opgeslagen is in je database.

Paco de Wulp

17-05-2015 23:15

Ik neem alle hints, suggesties, tips mee. Wat een fantastische website is dit toch !

Onbekende gebruiker

18-05-2015 22:22

AJAX is ook maar een soort 2e 'headless' browser die op de achtergrond HTTP requests doet, net als de browser. Alleen dan gebeurt het met een XMLHTTPRequest (XHR) object, in plaats van dat de gebruiker een URL typt/klikt/afvuurt. Voor Javascript hoeft er verder niets speciaals gedaan te worden, zolang je de inputvariabelen maar controleert en schoont voordat je ze bij een SQL query in stopt. Hiervoor gelden dus dezelfde regels, en kan ik prepared statements aanbevelen.

Reageren

Inloggen om te reageren