SQL Injection in PostgreSQL verhindern

[Hans]

Hallo,
ich möchte eine Applikation mit PostgreSQL und PHP schreiben und möchte zuerst einmal die Sicherheit abklopfen (newbe ;-).

Wenn ich Variablen auf einer Seite entgegennehme um diese in meinem SQL zu verwenden will ich natürlich SQL Injecten verhindern. Dazu habe ich z.B. folgende Funktion gefunden:

  
//injection Test  
function SQLInjectionTest($checkstring){  
	$sqltest = array ("/SELECT.*FROM.*WHERE/i",  
	"/INSERT.*INTO/i",  
	"/DELETE.*FROM/i",  
	"/UPDATE.*WHERE/i",  
	"/ALTER.*TABLE/i",  
	"/DROP.*TABLE/i",  
	"/CREATE.*TABLE/i",  
	"/substr/i",  
	"/varchar/i",  
	"/or.*\d=\d/i",  
	"/and.*\d=\d/i");  
	foreach ($sqltest as $regex){  
		if (preg_match($regex, $checkstring)){  
			return TRUE;  
		}  
	}  
	return FALSE;  
}  

Damit überprüfe ich zunächst die Get Variable auf Schadcode und ecsape danach noch den String mit htmlspecialchars.

  
if (SQLInjectionTest($_GET['service'])){  
	echo "<h1> Unerlaubte Eingabe! </h1>";  
	exit(0);  
}  
  
$sqlString = "SELECT * FROM mytab WHERE service ='".htmlspecialchars($_GET["service"])."'";  
$result_service = pg_query($dbconn , $sqlString) or die('Connection failed (pg_query): ' . pg_last_error());  
  

Wäre das so schon ausreichend gesichert oder muss ich das anders machen?
Danke für Eure Hilfe!
Hans

[dedlfix]

Tach!

Wenn ich Variablen auf einer Seite entgegennehme um diese in meinem SQL zu verwenden will ich natürlich SQL Injecten verhindern.

Das ist nicht der richtige Ort, um so etwas zu tun. Die Eingabedaten sollten lediglich auf fachliche Anforderungen überprüft werden. Natürlich ist es sinnlos, Statementbestandteile in der Datenbank vorzufinden, aber da gibt es noch genügend anderen Müll, den man filtern müsste. Solch ein Filter wäre dann aber ein generelles (nicht komplett lösbares) Problem und nicht nur auf Statementbestandteile einzuschränken.

Dazu habe ich z.B. folgende Funktion gefunden:

//injection Test
function SQLInjectionTest($checkstring){
$sqltest = array ("/SELECT.*FROM.*WHERE/i",
"/INSERT.*INTO/i",
"/DELETE.*FROM/i",
"/UPDATE.*WHERE/i",
"/ALTER.*TABLE/i",
"/DROP.TABLE/i",
"/CREATE.TABLE/i",
"/substr/i",
"/varchar/i",
"/or.\d=\d/i",
"/and.\d=\d/i");
foreach ($sqltest as $regex){
if (preg_match($regex, $checkstring)){
return TRUE;
}
}
return FALSE;
}

> Damit überprüfe ich zunächst die Get Variable auf Schadcode  
  
Und du bist sicher, dass du auf diese Weise wirklich alles Gefährliche erwischst (für jetzt und in alle Ewigkeit)?  
  

> und ecsape danach noch den String mit htmlspecialchars.  
  
Ein SQL-Statement ist kein HTML. Eine Aufbereitung für HTML ist daher nicht sinnvoll. Das kommt erst dann ins Spiel, wenn du irgendwas nach HTML ausgeben möchtest - und auch erst dann und nicht x Verarbeitungsschritte vorher.  
  

> Wäre das so schon ausreichend gesichert oder muss ich das anders machen?  
  
Anders. Verwende nur die für Postgres vorgesehenen Maskierfunktion pg\_escape\_\*(), vor allem [pg_escape_string()](http://php.net/manual/en/function.pg-escape-string.php) und [pg_escape_literal()](http://php.net/manual/en/function.pg-escape-literal.php).  
  
  
dedlfix.

[Hans]

Hi, danke für die Hilfe!

Wäre das so schon ausreichend gesichert oder muss ich das anders machen?

Anders. Verwende nur die für Postgres vorgesehenen Maskierfunktion pg_escape_*(), vor allem pg_escape_string() und pg_escape_literal().

Wäre dann sowas o.k.?

  
  
if (isset($_GET['service']) && $_GET['service'] != "") {  
	$getservice = pg_escape_string($_GET["service"]);  
	  
	$sqlString = "SELECT * FROM mytab WHERE service ='".$getservice."'";  
	$result_service = pg_query($dbconn , $sqlString) or die('Connection failed (pg_query): ' . pg_last_error());  
}  
  

Hans

[dedlfix]

Tach!

Wäre dann sowas o.k.?

if (isset($_GET['service']) && $_GET['service'] != "") {

$getservice = pg_escape_string($_GET["service"]);

$sqlString = "SELECT * FROM mytab WHERE service ='".$getservice."'";
$result_service = pg_query($dbconn , $sqlString) or die('Connection failed (pg_query): ' . pg_last_error());
}

  
Aus SQL-Injection-sicherheitstechnischen Gesichtspunkten ist das ok. Mit \_literal() statt \_string() kannst du sogar noch die Anführungszeichen drumherum einsparen.  
  
Das "or die()" ist aber nicht nur aus Sicht der Benutzerfreundlichkeit beanstandenswert. Ein Normal-Anwender kann mit der Fehlermeldung nichts anfangen und andere als der Administrator sollen damit nichts anfangen können. Also ist die Ausgabe in Richtung Client nicht empfehlenswert. Und ein Abbruch statt einem für den Fall angemessenen alternativen Abschluss des Vorgangs kann geschäftsschädigend sein, weil zum Beispiel Kunden dann anderswo kaufen gehen.  
  
Weiterhin lässt sich das isset mit Nicht-leer-Prüfung durch ein empty() ersetzen.  
  
  
dedlfix.