GPC-Arrays auf SQL-Injection etc. prüfen

[Tobias Unger]

Hi!

Da ich mir noch nicht ganz sicher bin und bei solchen Fragen lieber ein mal zu oft nachfrage als ein mal zu wenig wollte ich mal fragen, was ich machen muss, um SQL-Injection sowie Angriffe auf das PHP-Script per GPC-Daten zu verhindern?
Reicht da eine Schleife, die am Anfang des Scripts alle GPC-Daten nach " und ' durchsucht und gegenenfalls eine Fehlermeldung ausgibt und das Script abbricht oder kann man noch mit mehr Zeichen Schaden anrichten?

Das Escapen aller Zeichen dann, wenn sie gebraucht werden mag zwar teilweise resourcensparender sein, hat aber auch seine Nachteile und ist ein ziemlicher Aufwand, der die Lesbarkeit des PHP-Codes meiner Meinung nach doch sehr beeinträchtigt (auch, wenn es nur ein Funktionsaufruf ist).

Von php-faq.de kenne ich (unter anderem) die Artikel:
http://php-faq.de/q/q-sql-injection.html
http://php-faq.de/q/q-sicherheit-parameter.html
http://php-faq.de/q/q-security-variablen.html

Danke schon einmal für eure Bemühungen.

Freundliche Grüße aus Nürnberg,
Tobias

[wahsaga]

hi,

was ich machen muss, um SQL-Injection sowie Angriffe auf das PHP-Script per GPC-Daten zu verhindern?

Um SQL-Injection zu verhindern, behandelst du jedes Datum, welches du in eine Query einbaust.
Und PHP ist über diese Datenquellen nur dann "angreifbar", wenn register_globals=on ist _und_ unsaubere Programmierung hinzukommt - also Variablen vor ihrer ersten Verwendung nicht sauber initialisiert werden.

Reicht da eine Schleife, die am Anfang des Scripts alle GPC-Daten nach " und ' durchsucht und gegenenfalls eine Fehlermeldung ausgibt und das Script abbricht

Da du in deinem Posting die beiden Zeichen " und ' verwendet hast, wäre wenn hier ein von dir geschriebenes Script laufen würde, dir das Posten dieser Frage also gar nicht möglich gewesen, sondern das Script wäre abgebrochen worden ...?
Nein, das kannst du nicht ernst meinen.

Das Escapen aller Zeichen dann, wenn sie gebraucht werden

... ist das einzig sinnvolle.

mag zwar teilweise resourcensparender sein, hat aber auch seine Nachteile und ist ein ziemlicher Aufwand, der die Lesbarkeit des PHP-Codes meiner Meinung nach doch sehr beeinträchtigt (auch, wenn es nur ein Funktionsaufruf ist).

Kann ich nicht finden.

Daten zu maskieren oder in sonsteiner Weise zu behandeln, wenn dies gar nicht erforderlich ist, ist jedenfalls Unfug.

<blockquote cite="Cheatah">
Das gilt übrigens _immer_, wenn Du eine Information in einen bestimmten Kontext bringst. Sie muss(!) immer(!) kontextspezifisch kodiert werden.
</blockquote>

Von php-faq.de kenne ich (unter anderem) die Artikel:
http://php-faq.de/q/q-sql-injection.html
http://php-faq.de/q/q-sicherheit-parameter.html
http://php-faq.de/q/q-security-variablen.html

Damit hast du ja schon mal ein paar gute Quellen an der Hand, die dir erklären, was du zu tun hast.

gruß,
wahsaga

[MudGuard]

Hi,

Und PHP ist über diese Datenquellen nur dann "angreifbar", wenn register_globals=on ist

Du meinst also, bei
mysql_query("DELETE FROM tabelle WHERE ID = ".$_POST['killme']);
ist ungefährlich, wenn register_globals=on ist?

Wenn also jemand einen POST-Request mit
killme=1 OR ID>0
an dieses Script schickt, kann es nicht zu unerwünschten Nebeneffekten kommen?

_und_ unsaubere Programmierung hinzukommt - also Variablen vor ihrer ersten Verwendung nicht sauber initialisiert werden.

Meines Erachtens reicht unsaubere Programmierung alleine aus, register_globals muß dazu nicht auf on stehen.

Das Escapen aller Zeichen dann, wenn sie gebraucht werden
... ist das einzig sinnvolle.

Nö - zusätzlich sollten auch noch Werte auf Sinnhaftigkeit geprüft werden - siehe obiges Beispiel, keines der Zeichen würde escaped werden.
Wenn - s.o. Beispiel - z.B. nur eine numerische ID erwartet wird, sollte auch überprüft werden, daß nur eine numerische ID kommt.

cu,
Andreas

[wahsaga]

hi,

Und PHP ist über diese Datenquellen nur dann "angreifbar", wenn register_globals=on ist

Du meinst also, bei
mysql_query("DELETE FROM tabelle WHERE ID = ".$_POST['killme']);
ist ungefährlich, wenn register_globals=on ist?

Du meinst off ;-)

Ich bezog mich auf die Frage, ob PHP damit "angreifbar" wäre.

Wenn also jemand einen POST-Request mit
killme=1 OR ID>0
an dieses Script schickt, kann es nicht zu unerwünschten Nebeneffekten kommen?

Doch, du hast natürlich recht.
Dies wäre dann aber auch wieder eher eine SQL-Injection, und kein Problem für PHP.

Nö - zusätzlich sollten auch noch Werte auf Sinnhaftigkeit geprüft werden - siehe obiges Beispiel, keines der Zeichen würde escaped werden.
Wenn - s.o. Beispiel - z.B. nur eine numerische ID erwartet wird, sollte auch überprüft werden, daß nur eine numerische ID kommt.

Ja, du hast natürlich Recht.

gruß,
wahsaga

[André Laugks]

Hallo!

Das Escapen aller Zeichen dann, wenn sie gebraucht werden mag zwar teilweise resourcensparender sein, hat aber auch seine Nachteile und ist ein ziemlicher Aufwand, der die Lesbarkeit des PHP-Codes meiner Meinung nach doch sehr beeinträchtigt (auch, wenn es nur ein Funktionsaufruf ist).

Sicherheit ist immer lässtig und zeitaufwendig! Zeichenketten zu escapen zieht nicht das System runter. Das ist eine banale Stringfunktion.

Von php-faq.de kenne ich (unter anderem) die Artikel:

Was sagen die Artikel alle aus? Die Welt ist schlecht! Vertrauen ist gut, Kontrolle ist besser!

Dich kann ruhig ein User Deine Scripte mit GPC-Daten befeuern! Bevor Du aber Daten aus GPC verwendest, musst Du sie prüfen, ob die Daten Deinen Mindestanforderungen gerecht werden.

Wenn Du eine ID erwartest, prüfe ob es sich um eine Ganzzahl handelt. Und wenn die ID größer 1000 seien musst, dann fragst Du auch noch ab, ob die ID größer 1000 ist. Wenn das nicht so ist, zeigst Du eine Fehlermeldung an oder lässt es ins Leere laufen.

Wenn Du ein String erwartest, dann escapst Du den String, bevor Du das SQL-Statement zusammen baust. Und wenn bestimmte Zeichen nicht vorkommen dürfen, überprüfst Du das!

Eigentlich ganz einfach!

André Laugks

[Auge]

Hallo

Da ich mir noch nicht ganz sicher bin und bei solchen Fragen lieber ein mal zu oft nachfrage als ein mal zu wenig wollte ich mal fragen, was ich machen muss, um SQL-Injection sowie Angriffe auf das PHP-Script per GPC-Daten zu verhindern?

Arbeite, jenachdem, woher du die Daten erwartest mit den superglbalen Arrays $_POST, $_GET, $_COOKIE, $_SESSION. Damit stellst du erstmal die Quelle klar. Je nachdem, um welche Art von Daten es geht, sind weitere Prüfungen angesagt.

Beispiele:

• Ein übergebener Wert aus einem <select> eines Formulars _muss_ aus einer fest definierten Menge von Möglichkeiten stammen. Du kannst den Wert also gegen ein Array der Möglichkeiten prüfen.

  
// Der Wert kommt per POST aus dem <select name="obst">  
  
// Das Array mit den moeglichen Werten, mit dem  
// auch das Formular gefuettert werden koennte.  
$select_obst = array("Apfel","Birne","Pflaume");  
  
if (isset($_POST["obst"]) and in_array($_POST["obst"],$select_obst))  
   {  
   // Der uebergebene Wert ist im Array vorhanden, er wird weiterverarbeitet.  
   // Ist er nicht vorhanden wird $_POST["obst"] ignoriert. Man koennte das  
   // Element auch bei Vorhandensein explizit loeschen, so es notwendig ist.  
   }  

• Ähnliches gilt für (Ganz)Zahlen. Da der übergebene Wert ein String ist, wird aber nicht auf is_int sondern auf is_number geprüft. Das geht übrigens auch mit einem regulären Ausdruck. Muss die Zahl in einem bestimmten Bereich liegen, kann (zusätzlich) wieder die Prüfung gegen ein Array erfolgen.

  
// Wir erwarten die Angabe eines Datums (Tag des Monats)  
  
// Monat und Jahr sind bereits bekannt, somit wissen wir,  
// wieviele Tage der Monat haben kann (wegen Schaltjahr).  
// Monat: Februar, Jahr: 2000  
  
// timestamp fuer den 01.02.2000, 00:00:00  
$validdate = mktime(0,0,0,2,1,2000);  
// Ermitteln der Anzahl der Tage des Monats (29)  
$monatstage = date("t",$validdate);  
// Array mit allen moeglichen Werten (1 bis 29)  
$array_mtage = range(1,$monatstage);  
  
if (isset($_POST["tag"]) and is_number($_POST["tag"]) and in_array($_POST["tag"],$array_mtage))  
   {  
   // $_POST["tag"] existiert, ist numerisch und eine gueltige Angabe.  
   }  

Für solche Dinge kann man sich Standardvorgehensweisen schaffen. Alles, was nicht den Erwartungen entspricht, wird von vornherein verworfen.

Jetzt kommt der freie Text. Der lässt sich nicht so einfach prüfen. Da heißt es, Schadensvermeidung durch Entschärfung.

Reicht da eine Schleife, die am Anfang des Scripts alle GPC-Daten nach " und ' durchsucht und gegenenfalls eine Fehlermeldung ausgibt und das Script abbricht oder kann man noch mit mehr Zeichen Schaden anrichten?

Je nach weiterer Verwendung können auch andere Zeichen als " und ' maskiert werden müssen. So ist z.B. zur Speicherung von Daten mit MySQL mysql_real_escape_string() für die Maskierung zuständig. Die Funktion "weiß am besten", welche Zeichen maskiert werden müssen. Da selbst mit addslashes und eigenen Prüfungen rumzukrepeln, ist Unsinn, wenn es dafür bereits eine fertige Funktion gibt.

Das Escapen aller Zeichen dann, wenn sie gebraucht werden mag zwar teilweise resourcensparender sein, hat aber auch seine Nachteile und ist ein ziemlicher Aufwand, der die Lesbarkeit des PHP-Codes meiner Meinung nach doch sehr beeinträchtigt (auch, wenn es nur ein Funktionsaufruf ist).

Wenn du deine Skripte sicher machen willst, kommst du um diese Funktionsaufrufe nicht herum. Und zwar genau dann, wenn sie erforderlich sind. Meiner Meinung nach stören sie auch die Lesbarkeit nicht, sofern der Quelltext eines Skriptes überhaupt lesbar formatiert ist.

Tschö, Auge

[dedlfix]

echo $begrüßung;

Das Escapen aller Zeichen dann, wenn sie gebraucht werden mag zwar teilweise resourcensparender sein, hat aber auch seine Nachteile und ist ein ziemlicher Aufwand, der die Lesbarkeit des PHP-Codes meiner Meinung nach doch sehr beeinträchtigt (auch, wenn es nur ein Funktionsaufruf ist).

Das letzte Argument ist ein ziemlich blödes, da es einigermaßen elegant umgangen werden kann.

Statt

$sql = 'INSERT INTO table (feld1,feld2) VALUES ("' . mysql_real_escape_string($_POST['feld1']) . '", "' . mysql_real_escape_string($_POST['feld2']) . '")';

was durch das ewige Raus-aus-dem-String/Rein-in-den-String zugegebenermaßen nicht unbedingt leicht lesbar ist könnte man

$sql = sprintf('INSERT INTO table (feld1,feld2) VALUES ("%s", "%s")',
    mysql_real_escape_string($_POST['feld1']),
    mysql_real_escape_string($_POST['feld2']));

schreiben. Oder man könnte die Möglichkeiten modernerer Datenbankschnittstellen nutzen. Beispiel mysqli:

$stmt = $mysqli->prepare('INSERT INTO table (feld1,feld2) VALUES (?, ?)');
  $stmt->bind_param('ss', $_POST['feld1'], $_POST['feld2']);

Hier kümmern sich die mysqli-Funktionen selbst sowohl um das Einrahmen von String-Werten als auch um die Entschärfung von kritischen Zeichen darin. Bei PDO sieht die Geschichte ähnlich aus (Example 6 und folgende).

Oder man schreibt sich einen eigenen Abstraktionslayer, der das gesamte Datenbankgeraffel von der eigentlichen Aufgabe des Scripts trennt, so das nur noch ein übersichtliches

$daten = array(
  'feld1' => $_POST['feld1'],
  'feld2' => $_POST['feld2']);
try {
  $datenbankhandle->insert('table', $daten);
} catch (Exception $ex) {
  // Fehlerbehandlung
}

übrigbleibt.

echo "$verabschiedung $name";

[Tobias Unger]

Hi!

Danke für eure zahlreichen Antworten.
Nun habe ich noch ein sehr vergleichbares Problem: Ich würde gerne eine ganze Menge Code übernehmen, der allerdings einfach immer die Variablen einsetzt, ohne sie zu escapen.
Gibt es hier eine Möglichkeit in die ohnehin anfangs immer per include() eingebundene Datei so etwas zu schreiben:

<?php
$postkeys = array_keys($_POST);
foreach($postkeys as $key) {
   $_POST[$key] = mysql_real_escape_string($_POST[$key]);
}
?>

Oder ist das reichlich sinnlos?

Freundliche Grüße aus Nürnberg,
Tobias

[dedlfix]

echo $begrüßung;

Nun habe ich noch ein sehr vergleichbares Problem: Ich würde gerne eine ganze Menge Code übernehmen, der allerdings einfach immer die Variablen einsetzt, ohne sie zu escapen.

Bist du dir sicher, dass du solchen fehlerhaften Code übernehmen willst? Wenn der Autor schon darauf nicht geachtet hat, dann ... aber das wäre nur Spekulation.

Gibt es hier eine Möglichkeit in die ohnehin anfangs immer per include() eingebundene Datei so etwas zu schreiben:

$postkeys = array_keys($_POST);
foreach($postkeys as $key) {
   $_POST[$key] = mysql_real_escape_string($_POST[$key]);
}
Oder ist das reichlich sinnlos?

Können kann man schon, besser als nichts ist es schon. Empfehlenswert wäre noch, die Konfiguration von magic_quotes_gpc zu beachten und die gegebenenfalls automatisch eingefügten Slashes vor dem mysql_real_escape_string()-Aufruf zu entfernen.

echo "$verabschiedung $name";

[Tobias Unger]

Hi!

Bist du dir sicher, dass du solchen fehlerhaften Code übernehmen willst? Wenn der Autor schon darauf nicht geachtet hat, dann ... aber das wäre nur Spekulation.

Ja - weil ich nicht mal eben ein halbes Jahr in neuen Code investieren will - da kümmere ich mich lieber 2 Tage um die paar Sicherheitslücken (das ist zwar nicht optimal - aber ich habe es einfach mal abgewägt und das ist so viel Arbeit, dass ich es nicht neu schreiben werde.

Können kann man schon, besser als nichts ist es schon. Empfehlenswert wäre noch, die Konfiguration von magic_quotes_gpc zu beachten und die gegebenenfalls automatisch eingefügten Slashes vor dem mysql_real_escape_string()-Aufruf zu entfernen.

Besser als nichts = "Suboptimal"?
Wo ist das Problem dabei? Wäre es denn theoretisch besser, das an jeder Stelle einzeln hinzuschreiben, wenn eine GPC-Variable genutzt wird? Wo wäre der Unterschied?

magic_quotes_gpc = on ; (ja, vielleicht nicht ganz optimal - aber ich kann es derzeit nicht ändern)

<?php
$postkeys = array_keys($_POST);
foreach($postkeys as $key) {
   $_POST[$key] = mysql_real_escape_string(stripslashes($_POST[$key]));
}
?>

Freundliche Grüße aus Nürnberg,
Tobias

[Alexander Brock]

Hallo Freunde des gehobenen Forumsgenusses,

Besser als nichts = "Suboptimal"?

Ja.

Wo ist das Problem dabei? Wäre es denn theoretisch besser, das an jeder Stelle einzeln hinzuschreiben, wenn eine GPC-Variable genutzt wird? Wo wäre der Unterschied?

Wenn du alle Benutzer-Eingaben pauschal für die Verwendung in MySQL-Statements maskierst bekommst du die ganzen Slashes auch in irgendwelche Vorschau-Ausgaben, Affenformulare etc.

Das sieht dann ziemlich doof aus und ist für Nicht-Programmierer nicht nachvollziehbar, ich würde also den gesamten Quellcode durchgehen und ihn korrigieren.

Gruß
Alexander Brock

[Tobias Unger]

Hi!

Das sieht dann ziemlich doof aus und ist für Nicht-Programmierer nicht nachvollziehbar, ich würde also den gesamten Quellcode durchgehen und ihn korrigieren.

Natürlich - irgend wo musste ja ein Hacken sein. Irgend wie hat es mich schon gewundert, dass das so einfach gehen sollte.
Das kann ich schon alleine desshalb nicht machen weil ich da eine PDF-Ausgabe ergänze und ...
Dann weis ich ja jetzt, was ich am Wochenende mache - wird bestimmt spannend, überall diesen Funktionsaufruf zu ergänzen... .

Trotzdem (bzw. gerade desshalb) vielen Dank!

Freundliche Grüße aus Nürnberg,
Tobias

[Auge]

Hallo

Natürlich - irgend wo musste ja ein Hacken sein.

Zumindest an jedem Fuß einer. ;-)

Das kann ich schon alleine desshalb nicht machen weil ich da eine PDF-Ausgabe ergänze und ...
Dann weis ich ja jetzt, was ich am Wochenende mache - wird bestimmt spannend, überall diesen Funktionsaufruf zu ergänzen... .

Überall dort und nur dort, wo MySQL in's Spiel kommt.

Tschö, Auge