SQL-Injections mit Lazarus möglich - Wo ist da der Bug?

[MmVisual]

Hallo allerseits,

In meinem Programm nutze ich Lazarus 2.7.1, nahezu das aktuellste Snapshot und als Datenbankkomponente ZEOS. Nun hat ein User festgestellt dass relativ einfach sich einen SQL Code als Injection mit TDBEdit und der verbundenen Query ausführen lassen!

Das was in der TDBEdit eingegeben wird ist einfach über TDatasource und TZQuery in die Datenbank, ohne extra Code geschrieben. (MySQL)

hier sind ein paar Screenshots.

1) Wenn ein Bauteil angelegt wird, das ein \' im Namen hat, wird ein SQL-Syntaxfehler gemeldet.

2) Wenn man den Bauteilnamen mit einem SQL-Kommentar enden lässt, ist die Syntax der SQL-Statements wieder korrekt (typisches SQL-Injection-Szenario: http://xkcd.com/327/" onclick="window.open(this.href);return false; )

3) Einfügen von OR TRUE in eine SQL-WHERE-Klausel. Die interne Abfrage (separate Query-Abfrage in BeforePost() ), ob ein Datensatz mit diesem Namen schon existiert gibt damit fälschlicherweise true zurück.

4) Mit Klick auf OK springt man zum ersten Datensatz, weil ein Bauteil dieses Namens doch noch nicht existiert.

Wo ist nun der Bug? In der Zeos Komponente oder in Lazarus?

Und die allerwichtigste Frage: Wie kann ich das beheben?

Grüße Markus

[MmVisual]

Ich denke, ich habe das gefunden, aber ich habe noch nicht die richtige Lösung für das Problem.

Wenn ein neuer Bauteilname eingegeben wurde, dann wird vor dem Speichern die Bezeichnung in eine Query genommen und in der Datenbank abgefragt ob es ein doppelter Eintrag gibt.
Und genau diese Query macht das Problem.

Code: Alles auswählen

q.SQL.Text := 'SELECT ID FROM bauteil WHERE Bezeichnung = ''' + qBauteilBezeichnung.AsString + '''';

Nun ist die Frage, wie mache ich das richtig?
Einfach Quoten reicht ja nicht.

Grüße Markus

[grl]

Wo ist nun der Bug? In der Zeos Komponente oder in Lazarus?

Also, ganz aus dem Bauch raus würd ich sagen: bei dir bzw bei dem was du programmiert hast.

Wie erzeugst du denn die Query? Wenn du ein "\'" in deinem Text zulässt führt das wahrscheinlich dazu, daß du mit dem Backslash ein Hochkomma in deine Query escapesd. Und das macht natürlich aus deiner Query ganz was anderes. Daß das nicht passiert musst du verhindern, das wird dir keine Komponente abnehmen könnnen,

Gruß
Luggi

[Bauer321]

so genau kenne ich mich damit auch nicht aus aber prepared statements ist so eine Sache die mir dazu einfällt. Das ist glaube ich extra gemacht um SQL-Injections zu vermeiden ist aber nachdem was ich neulich mal gelesen hab auch schneller

[MmVisual]

"prepared statements" - Ja genau, sowas brauche ich.

Gibt es unter Zeos eine Funktion die mir einen String fertig SQL tauglich aufbereitet?

[MmVisual]

Ich habe das mal so umprogrammiert:

Code: Alles auswählen

q.SQL.Text := 'SELECT ID FROM bauteil WHERE Bezeichnung = :Bezeichnung';
q.Params.ParamValues['Bezeichnung'] := qBauteilBezeichnung.AsString;

Jetzt scheint es zu klappen.

[grl]

Ich habe das mal so umprogrammiert:

Code: Alles auswählen

q.SQL.Text := 'SELECT ID FROM bauteil WHERE Bezeichnung = :Bezeichnung';
q.Params.ParamValues['Bezeichnung'] := qBauteilBezeichnung.AsString;

Jetzt scheint es zu klappen.

Das über Parameter zu lösen ist sicher gescheiter als den Query-String selber zu bauen.

Gruß
Luggi

[MmVisual]

Na dann hab ich mal zu tun ... 30000 Codezeilen zu durchforsten.

[Bauer321]

"prepared statements" - Ja genau, sowas brauche ich.

Gibt es unter Zeos eine Funktion die mir einen String fertig SQL tauglich aufbereitet?

http://sourceforge.net/project/shownote ... _id=618508 also geben tuts sie auf jeden Fall wie man sie genau nutzt kann ich dir nicht sagen. Das mit den parametern sollte allerdings dazu führen, dass diese von Zeos mittels mysql_real_escape_string Escaped werden (ansonsten sehe ich den sinn in den Parametern nicht)

[af0815]

Na dann hab ich mal zu tun ... 30000 Codezeilen zu durchforsten.

Bevor du das noch zig mal machen musst, schau dir mal an, wie du ev. Deinen Code schon bei der Eingabe resistenter machen kannst.

Schau dir mal die Beispiele an.
http://de.wikipedia.org/wiki/SQL-Injection
http://www.unixwiz.net/techtips/sql-injection.html
http://www.heise.de/security/artikel/Giftspritze-270382.html

[Bauer321]

Na dann hab ich mal zu tun ... 30000 Codezeilen zu durchforsten.

Bevor du das noch zig mal machen musst, schau dir mal an, wie du ev. Deinen Code schon bei der Eingabe resistenter machen kannst.

Schau dir mal die Beispiele an.
http://de.wikipedia.org/wiki/SQL-Injection
http://www.unixwiz.net/techtips/sql-injection.html
http://www.heise.de/security/artikel/Giftspritze-270382.html

durch ein prepared statement sollten eigentlich alle Risiken beiseite sein. Ansonsten zuerst den String mit mysql_real_escape escapen

[af0815]

durch ein prepared statement sollten eigentlich alle Risiken beiseite sein. Ansonsten zuerst den String mit mysql_real_escape escapen

Das Risiko ist vermindert. Zuerst sollte immer verhindert werden, das überhaupt ungewollte Einträge gemacht werden. Das heisst entweder Benutzereingaben durch vorgefertigte Liste zu ersetzen bzw. dort wo der benutzer echt Eingaben machen muß, den iput entsprechend zu parsen und gewisse Begriffe und Zeichen einfach nicht anzunehmen bzw. den Benutzer darauf hinzuweisen, das seine Eingabe so nicht gewünscht ist.
Beispiel: keine Hochkommas, Steuerzeichen, Bindestriche, Escapezeichen, spezielle Worte.

Je mehr der Input geprüft wird, desto mehr wird verhindert, das falsche Sachen an die DB geliefert werden, zusätzlich wird der Benutzer auch bei den EIngaben (vernünftig) hingewiesen wenn was nicht so stimmt.

[MmVisual]

Das Problem ist ja auch nicht so einfach zu lösen.

Meine EXE läuft je nach Konfiguration mit SQLite, MySQL und PostgeSQL (über die ZEOS Komponente). Und bei allen Datenbanktypen dürfen keine SQL-Injections auftreten.

Ich habe jetzt das gesamte Programm durchforstet und die Stellen mal mit dieser Syntax ersetzt:

Code: Alles auswählen

q.SQL.Text := 'SELECT ID FROM bauteil WHERE Bezeichnung = :Bezeichnung';
q.Params.ParamValues['Bezeichnung'] := sBezeichnung;

Aber einfach mal so ins blaue hinein möchte ich den User nicht einschränken, es sollte dann schon was "Handfestes" sein, das ich abfangen muss.

Ab Montag testet ein User meine Änderung mit MySQL. Ich selbst habe zu wenig Erfahrung mit SQL-Injections.

[af0815]

Das Problem ist ja auch nicht so einfach zu lösen.

Meine EXE läuft je nach Konfiguration mit SQLite, MySQL und PostgeSQL (über die ZEOS Komponente). Und bei allen Datenbanktypen dürfen keine SQL-Injections auftreten.

Das glaube ich Dir.
Das einfachste wird sein, das du dir eine Funktion erstellst, wo du die wichtigesten Sachen (zu Fuß) prüfst (siehe vorherigen Post von mir). Damit sind mal die meisten Sachen für die Scriptkiddies blockiert.

[MmVisual]

Das "einfachste" ist nicht so einfach, daher warte ich lieber ab.

Denn ein TDBEdit >> TDatasource >> TZQuery Verbindung macht alles schon automatisch. Darin werden die Daten eingegeben, dargestellt und gespeichert.
Jetzt darin den eingegebene Text zu überprüfen und die Eingabe zu blockieren ist schon eine größere Sache.
Wenn das Probleme bereiten würde, dann wäre sicher JEDE Lazarus-Anwendung mit Datenbanken automatisch ein Sicherheitsrisiko und es wäre hier sicher schon breit diskutiert worden.

Daher lasse ich das erst mal auf "Prepared Statements".