sichere Datenbankabfragen - prepared Statements?

d3mueller · 24.05.2014

Hi, ich bin am überlegen, was die beste und unkomplizierteste Möglichkeit ist, die Queries für SQL sicher zu machen. Es muss nicht bis ins letzte durchdacht und unglaublich kompliziert und verstrickt sein^^.

Früher habe ich immer so was wie mysqli_real_escape_string benutzt. Aber jetzt bin ich auf "prepared statements" gestoßen. Das soll ja ziemlich sicher sein, zumindest habe ich das so gelesen.

Nur ist es so:

Bisher habe ich (für die "internen" Funktionen, auf dessen Werte die User keinen Einfluss haben) eine query()-Methode benutzt. Die geht einfach so, dass ich als ersten Parameter das SQL schicke und als 2. Parameter den Rückgabetyp, z.B. das Array mit den Inhalten aus der Datenbank.

Jetzt dann diese Prepared-Statements zu benutzen würde das ja komplett umstellen. Die query()-Methode könnte ich so ja gar nicht mehr benutzen.

Meine Fragen:

1. Ist es wirklich nötig, prepared statements zu verwenden? Oder ist das ein unnötiger Aufwand?Weil ich halt alles umbauen müsste und ich dann nicht mehr so "bequem" Queries ausführen könnte.

2. Bei mir funktioniert es nicht. Wenn ich schreibe:

$stmt = self::$database->prepare($sql);

Call to undefined method Database:

repare()

Ich weiß jetzt nicht, welche Stelle im Code dafür wichtig ist. Hier sind mal die beiden Datenbank-Klassen, die ich benutze. die erste, abstract, Klasse ist da, damit die ganzen "externen" Klassen Zugriff auf das Datenbank-Objekt haben. Also alle anderen Klassen werden damit erweitert

PHP:

class Buildings extends DatabaseBased {...}

PHP:

abstract class DatabaseBased {
   protected static $database;
 
    public static function setDatabase(Database $database)
    {
    self::$database = $database;
    }
}


class Database
{
   public $connection;
 
 
   public function __construct($host, $user, $password, $database) {
     $this->connection = mysqli_connect($host, $user, $password, $database);
     if (mysqli_connect_errno()) {
       throw new Exception(__METHOD__.'::'.mysqli_connect_error());
     }
     // Die jetzt erstellte Instanz wird in DatabaseBased gespeichert
     DatabaseBased::setDatabase($this);
   }
 
   public function close() {
     if (!$this->connection) return false;
     $this->connection->close();
   }
 
   public function query($sql, $return='affected', $result_mode=MYSQLI_USE_RESULT) {
     $data = array();
   
     if ($result = $this->connection->query($sql, $result_mode)) {
       if ($return=='affected'){ $data = $this->connection->affected_rows; }
       elseif ($return=='num'){ $data = mysqli_num_rows($result); }
       elseif ($return=='id'){ $data = $this->connection->insert_id; }
       elseif ($return=='assoc'){ while ($row = $result->fetch_assoc()) $data[] = $row; }
       elseif ($return=='numeric'){ while ($row = $result->fetch_assoc()) $row = $result->fetch_array(MYSQLI_NUM); }
       elseif ($return=='fields'){ while ($row = $result->fetch_fields()) $data[] = $row; }
       elseif ($return== 'ressources') { while($row = $result->fetch_assoc()) $data[$row["ressourceId"]] = $row; }
       else { while($row = $result->fetch_assoc()) $data[$row[$return]] = $row; }
       if (is_object($result)) $result->close();
     } else {
       throw new Exception(__METHOD__.'::'.$this->connection->error.'::'.$sql);
     }
     return $data;
   }
 
   public function startTransaction()
   {
    if (!$this->connection) return false;
    $ok = $this->query("SET AUTOCOMMIT=0");
    return ($ok && $this->query("START TRANSACTION"));
   }
   public function commit()
   {
    if (!$this->connection) return false;
    if (!$this->query("COMMIT")) return false;
    $this->query("SET AUTOCOMMIT=1");
    return true;
   }
   public function rollback()
   {
    if (!$this->connection) return false;
    if (!$this->query("ROLLBACK;")) return false;
    $this->query("SET AUTOCOMMIT=1");
    return true;
   }
 
}

Ich glaube, die Fehlermeldung kommt, weil ich das Datenbank-Objekt an sich anspreche, und die die eigentliche Verbindung zur Datenbank. Aber ich schaff es nicht, das hinzubekommen. Habt ihr Ideen?
Irgendwie will es heute absolut nicht klappen. Ich hock schon ewigkeiten davor und schaff es nicht. Hab auch schon im Internet geschaut. Heute ist absolut nicht mein Tag^^

Ich hoffe ich habe jetzt keine Angaben vergessen, falls ja, dann sorry^^Dann werde ich es noch nachreichen

Vielen Dank schon mal

LG

EDIT:

Okay, das oben beschriebene Problem habe ich gelöst. War gestern irgendwie zu blöd. War ganz einfach.

Das Problem, welches ich jetzt habe, ist folgende Fehlermeldung:

Call to undefined method mysqli_stmt::get_result()

Wenn ich schreibe

PHP:

$result = $stmt->get_result();

Habe gelesen, dass man dafür erst einen Treiber auf dem Webspace installieren muss. Das kann ich (glaube ich) bei strato nicht. Also fallen prepared statements ja schon weg :/

Gibt es noch andere Möglichkeiten?

JPS · 27.05.2014

Warum brauchst Du für ein prepared Statement einen Treiber?
Von der Definition sind das ja nur SQL Statements, die, weil sie öfters gebraucht werden, abgespeichert werden und nur die Parameter ausgetauscht werden.
Und soweit ich weiß, liefert die mysqli Klasse prepare als Methode mit. (http://www.php.net/manual/de/mysqli.prepare.php)
Hab ein Beispiel gefunden, was es eigentlich gut erklärt:

Code:

<?php
$stmt = $dbh->prepare("SELECT user, password FROM tbl_user WHERE (user=:user)");
$stmt->bindParam(':user', $user);

// eine Zeile abfragen
$user = 'Alice';
$stmt->execute();

// eine weitere Zeile mit anderen Werten abfragen
$user = 'Bob';
$stmt->execute();
?>

ovbb · 27.05.2014

ich persönlich arbeite mit "stored procedures". hab kA ob das MySQL unterstützt.
Besonders bei großen und komplexen Abfragen hab ich damit einiges an Performance heraushohlen können.

@btt: Ich würde immer auf prepared statements zurückgreifen. je nach programiersprache (verwende c#, muss leider für Windoof entwickeln) gibt es dabei unterschiedliche möglichkeiten.

ganz so wie JPS sehe ich das nicht. bei prepared statements wird auch der datentyp angegeben. also wenn eine parameter ein int sein soll dann liefert dies bei einem string als parameter eien exception. somit hat man auch typsicherheit (natürlich sollte man das auch ohnedies vorher überprüfen

).

JPS · 27.05.2014

@ovbb - Typensicherheit ist bei PHP ja eh so eine Sache. Als C# Mensch wird Dir bei den Thema in Bezug auf PHP bestimmt das Augenrollen komen

Aber prinzipiell widersprechen sich ja unsere Aussagen nicht und ich sehe das angeben eines Datentyps als zusätzlichen Vorteil der prepared Statements.

Kaja1309 · 27.05.2014

Da ich mich auch ein wenig mit MySQL beschäftige bin ich auf folgendes gestoßen:
http://stackoverflow.com/questions/8321096/call-to-undefined-method-mysqli-stmtget-result

das scheint als ob du mit evtl. nur mit den Methoden bind_result und fetch arbeiten müßtest.

Um für Sicherheit zu sorgen was SQL Statements angeht würde ich aber entsprechend die Eingaben prüfen bzw. escapen bevor sie in die entsprechenden Abfragen eingebettet werden, was Du ja bereits tatest. Stored Procedures/Functions sind seit MySQL 5 verfügbar, diese wären wahrscheinlich auch sicherer, lohnen sich aber eher in aspekt transaktionale Änderungen oder umfangreichere sequentielle Abfragen auf den Datenbankserver auszulagern und bei Bedarf transparent optimieren zu können.

Duddle · 28.05.2014

d3mueller schrieb:
1. Ist es wirklich nötig, prepared statements zu verwenden? Oder ist das ein unnötiger Aufwand?Weil ich halt alles umbauen müsste und ich dann nicht mehr so "bequem" Queries ausführen könnte.

Bequemlichkeit ist oft eine Ursache von Sicherheitslücken.

Es ist nicht notwendig, Prepared Statements zu verwenden. Sie unterbinden lediglich eine komplette Klasse von Angriffen, automatisch und ohne dass du darüber nachdenken musst.

Duddle

d3mueller · 29.05.2014

Okay, danke schön. Hab es jetzt hingekriegt

mit bind_results() und fetch()

Was mir jetzt noch eingefallen ist, ist folgendes Szenario:

Angenommen ich habe einen String, der vom User eingegeben wurde. Der String besteht aus einer SQL-Attacke. Also irgendein Code, der z.B. eine Tabelle löschen würde.

Nun speichere ich das über ein prepared Statement ab. So sollte ja nichts passieren.
Aber wenn ich jetzt diesen in der Datenbank gespeicherten String wieder auslese und dann ohne ein prepared Statement in einer anderen Datenbanktabelle speichere, was passiert dann? Ist der String dann immer noch gefährlich? Also jetzt rein theoretisch.

Oder erkennt der das direkt beim prepared statement, dass es sich um einen Hacking-Versuch handelt, und gibt z.B. ne Exception aus?

Ich kann es nicht wirklich ausprobieren. Hab es mal versucht, aber ich kenne mich null aus mit SQL Injection^^. Hab mal im Internet nachgeschaut, aber er speichert alles trotzdem nur als normalen String in der Datenbank und zwar 1:1 wie ich es eingetippt habe. Keine Ahnung, ob der Code falsch war, oder die prepared Statements das abgefangen haben

Kaja1309 · 29.05.2014

Bin da auch nicht so versiert, aber prinzipiell denke ich mir es macht Sinn die möglichen Eingaben auf die zu erwarteten Zeichen zu reduzieren wer braucht schon ", ', ; ... , und dieses besser nicht mit JavaScript auf der Page sondern im dem tatsächlich die SQL Statements generierenden und ausführenden Applikationen/Skripten.

Evtl. schau mal bei http://www.php.net/manual/de/security.database.sql-injection.php oder http://de.wikipedia.org/wiki/SQL-Injection falls noch nicht gesehen.

ovbb · 29.05.2014

hm ... das ist eine gute frage die man mal recherchieren sollte ...

persönlich speicher ich nichts in die datenbank ohne es zu überprüfen ... wenn kritische symbole escaped werden sollte das passen ... aber wie gesagt ist es zu probieren

blöder ist das ganze mit xss-attacken die du in deiner DB sabspeicherst und beim laden ggf. fall du die tabellenwerte einfach anzeigst, ausgeführt werden. hierbei gibt es verschiedene möglichkeiten die syntax zu verschleiern

Duddle · 29.05.2014

Wenn du einer Datenbank sagst "gib mir Wert foo aus Tabelle $variable" und $variable ist "bar", dann sagst du der Datenbank "gib mir Wert foo aus Tabelle bar".
Wenn ein Angreifer $variable auf "bar und gib mir Wert x aus Tabelle y" setzt, dann sagt das der Datenbank "gib mir Wert foo aus Tabelle bar und gib mir Wert x aus Tabelle y". Mehr ist eine SQL-Injection ja nicht.

Ein Prepared Statement macht folgendes: du sagst der Datenbank "gib mir Wert foo aus Tabelle $variable". Diese bereitet sich darauf vor, den Wert foo aus der Tabelle zu holen, die du später in $variable einträgst. Der Abfrageweg ist also komplett vorbestimmt und kann nicht mehr verlassen werden.
Wenn ein Angreifer nun $variable auf "bar und gib mir Wert x aus Tabelle y" setzt, dann versucht die Datenbank, aus Tabelle "bar und gib mir Wert x aus Tabelle y" den Wert foo zu lesen. Da der Abfrageweg nicht mehr verändert werden kann, versucht die Datenbank auch nicht den zusätzlichen Befehl auszuführen.

Wenn du nun den Wert "bar und gib mir Wert x aus Tabelle y" in $andereVariable abspeicherst und später die Datenbank ohne Prepared Statement fragst "gib mir Wert foo aus Tabelle $andereVariable", dann versteht sie wieder "gib mir Wert foo aus Tabelle bar und gib mir Wert x aus Tabelle y".

Vertraue niemals Nutzereingaben, selbst wenn du diese kurzzeitig zwischenspeicherst.

Duddle

d3mueller · 29.05.2014

danke

Also muss man immer aufpassen.

Ist es dann sinnvoll, zu escapen und zusätzlich noch prepared statements zu verwenden? Weil wenn man einfach überall prepared statements benutzt, dann könnte man sich das ja sparen, oder? Dann hat man auch keine Probleme mit den Zeichen " oder '

Duddle · 29.05.2014

d3mueller schrieb:
Weil wenn man einfach überall prepared statements benutzt, dann könnte man sich das ja sparen, oder?

Jap.

Duddle

d3mueller · 31.05.2014

Okay, ich habe das jetzt mal ne Weile getestet. Und es funktioniert auch immer alles, nur ist jede Datenbankabfrage ziemlich aufwendig. Weil man immer sowas schreiben muss:

PHP:

$sql = "SELECT id FROM table WHERE name=? LIMIT 0,1";
             $stmt = self::getConnection()->prepare($sql);
             $stmt->bind_param('s', $name);
             $stmt->execute();
             $stmt->bind_result($id);
             $stmt->store_result();
             $stmt->fetch();

Für jede einzelne Abfrage. Ist das schlimm? Dauert das nicht (verhältnismäßig) ewig, wenn er immer so viele Befehle in der Datenbank ausführen muss?

Duddle · 31.05.2014

d3mueller schrieb:
Und es funktioniert auch immer alles, nur ist jede Datenbankabfrage ziemlich aufwendig. Weil man immer sowas schreiben muss:

Oder du abstrahierst das Konzept und erstellst eine Klasse mit entsprechenden Methoden. So wie man es immer macht, wenn ein Ablauf mehrmals identisch durchgeführt wird. Es gibt auch fertige Wrapper-Klassen, aber es ist immer eine gute Übung sowas selbst zu erstellen.

d3mueller schrieb:
Dauert das nicht (verhältnismäßig) ewig, wenn er immer so viele Befehle in der Datenbank ausführen muss?

Es ist ineffizienter, wenn du es ineffizient benutzt. Führst du die gleiche Abfrage mit mehreren Variablen durch, muss sie bspw. nur einmal vorbereitet werden.
Abgesehen davon, du schliesst sicher auch deine Wohnung ab, obwohl es dann minimal länger dauert, in deine Wohnung zu kommen.

Duddle

d3mueller · 31.05.2014

Ja, daran habe ich mich schon versucht, aber ich habe ein Problem:

PHP:

public static function databaseAccess($sql, $bindParameter, $bindResult) {
$stmt = self::getConnection()->prepare($sql);
$stmt->bind_param('s', $bindParameter);
$stmt->execute();
$stmt->bind_result($bindResult);
$stmt->store_result();
}

Irgendwie so in der Art, aber ich habe ja unterschiedlich viele $bindParameter und $binResult. Wenn ich mehrere Spalten in der Tabelle auswähle, brauche ich mehr $bindResult z.B.

Und bei $bindParameter muss ich auch noch sagen, ob es ein integer oder string ist.

Mit nem Array geht das ja nicht, weil man ja den $variablennamen an sich dort angeben muss, nicht dessen inhalt. Deswegen komme ich da nicht weiter

Duddle schrieb:
Es ist ineffizienter, wenn du es ineffizient benutzt. Führst du die gleiche Abfrage mit mehreren Variablen durch, muss sie bspw. nur einmal vorbereitet werden.
Abgesehen davon, du schliesst sicher auch deine Wohnung ab, obwohl es dann minimal länger dauert, in deine Wohnung zu kommen.

Okay, das ist ein gutes Argument