Wie SQL-Injection verhindern?

[Amargedd0n]

Hallo Leute,

ich bin nun endgültig fertig mit dieser Script und hab da noch eine Frage:

In meinem Script kann man einen Account löschen per Klick auf "Delete". Der Account hat die ID beispielsweise 2. also lautet meine URL, wenn man auf Delete Klickt "delete.php?id=2". Funktioniert wunderbar, man wird auf delte.php weitergeleitet sozusagen, der Acc wird gelöscht und fertig. Doch so eifanch ist das nicht.
Ich hab einfach mal zum Test auf ne SQL-Injection überprüft und habe sofort eine gefunden, was ya nicht so toll ist. Leider weiß ich nur grad ehrlich gesagt nicht, wie ich die entferne :S
Hier mal der Source Code:

delete.php

<?php 
include ('Inc/config.php');
include ('Template/header.html');
include ('Template/Menu.html');

$db = mysql_connect($host,$username,$password,$dbname);

//Datenbank auswaehlen
mysql_select_db($dbname,$db);

$id = mysql_real_escape_string($_GET['id']);
 

$sql = 'DELETE FROM acclister 
                WHERE ID='.$id;
                
 $result = mysql_query($sql);

if (!$result) {
die ('Es ist ein Fehler aufgetreten: '.mysql_error().'!');
}

echo '<img src = img/ico/ok.PNG>Der Account mit der ID = '.$id.' wurde erfolgreich gel&ouml;scht!';

//Footer Einfugen
include ('Template/footer.html');
 ?>

$id wird von der URL "delete.php?id=2" übergeben nur so als nebeninfo.

Danke euch
Amar

[Frank]

Hier kann dir [MAN]intval[/MAN] helfen, um sicherzugehen, dass du im SQL eine Zahl bekommst. Trotzdem sollte dein Script mit [MAN]mysql_real_escape_string[/MAN] bereits abgesichert sein. Was verstehst du denn genau unter SQL-Injection in diesem Fall?

[Manko10]

Was heißt, du hast direkt eine gefunden? Du meinst, du hast eine Lücke gefunden, die es ermöglicht, SQL-Injections auszuführen?
An sich ist dein Ansatz, um eine Injection zu verhindern, schon nicht schlecht, eine kleine Anmerkung aber noch:

$sql = 'DELETE FROM acclister 
                WHERE ID='.$id; 

Wenn du für den Wert von $id keine Quotes setzt, also keinen String erwartest, sondern einen Integer, dann solltest du auch auf einen Integer prüfen, denn ansonsten können geschickte Leute z.B.

1 OR 1 = 1

übergeben und aus deinem Statement wird

DELETE FROM acclister WHERE ID = 1 OR 1 = 1

und das löscht alles aus der Tabelle. mysql_real_escape_string() kann daran nichts ändern, weil keine Stringdelimiter zum Maskieren da sind. Also entweder Quotes setzen:

$sql = 'DELETE FROM acclister WHERE ID="'.$id.'"'; 

oder direkt auf einen String prüfen:

if ((string)((int)$_GET['id']) !== $_GET['id']) {
    exit('Fehlerhafter Wert!');
}
$id = $_GET['id']; 

oder direkt filtern, je nach Anwendungsfall:

$id = (int)$_GET['id']; 

Beitrag editiert:
[…] @Frank: wie ich erläuterte, existiert in diesem Fall wirklich ein Lücke.

[Screeze]

ich schätze er meint, dass wenn man statt einer zahl einen text also

delete.php?id=irgendwas oder so

eingibt, dass dann sein sql fehlerhaft wird, weil er seine id nicht in ' ' setzt im query. Aber durch prüfung mit intval, oder einfach (int) $id, sollte das behoben sein.

edit: mist manko war schneller

[Manko10]

Kleiner Nachtrag: die Validierungsmethode ist die sauberste, die Filtermethode ein gesunder Kompromiss. Die Ich-Schau-Mal-Was-Da-Kommt-Kann-Ja-Wegen-Der-Quotes-Nichts-Passieren-Variante ist eher das faule Ei zu Ostern.

[ByStones]

Was du auch noch einbauen solltest ist, dass man nur seinen eigenen Account löschen darf, weil bei deinem jetzigen Code kann man jede ID übergeben und wild Accounts löschen...

[Capfly]

Hier kann dir [MAN]intval[/MAN] helfen, um sicherzugehen, dass du im SQL eine Zahl bekommst. Trotzdem sollte dein Script mit [MAN]mysql_real_escape_string[/MAN] bereits abgesichert sein. Was verstehst du denn genau unter SQL-Injection in diesem Fall?

Ich hatte ihm id schon escaped, er weiß wohl noch nicht ganz, wie das läuft^^

[Manko10]

IDs zu escapen bringt nicht viel, wenn auch ohne Quotes die Möglichkeit besteht, SQL-Injections auszuführen. Siehe mein Posting weiter oben.

[Capfly]

Hmm, habe ich noch nicht bedacht...
Gut, dass bei meinem MySQL ausgelesene Daten nicht angezeigt werden ^.^

[Amargedd0n]

Habsmalso versucht:

if(!intval($id)) {
die('Versuch abgewehrt');
} 

Klappt auch nicht leider Wenn ich ein ' hinter die ID mache, dann kommt halt ein SQL-Error, weil das Query zerstört wird.... aber ich weiß nicht wie ich das beheben soll

Edit: Habs mal mitdem intval(); in das Query gepackt, so dass es so aussieht:

$sql = 'DELETE FROM acclister 
                WHERE ID='.intval($id); 

Jetzt krieg ich keinen Error mehr, aber wenn ich die Meldung kriege, dass der Account erfolgreich gelöscht wurde steht sowas da:

Der Account mit der ID = 8\\\' wurde erfolgreich gelöscht!

Guckt euch mal das markierte an.Meint ihr, mein Script ist sicher? :S

[Capfly]

Wo ein ' ?

[Asipak]

Sieht nach PHP: Laufzeit-Konfiguration - Manual aus.

[Amargedd0n]

Wo ein ' ?

hinter delete.php?id=2 also das es so aussieht:

delete.php?id=2'

das zerstört das mysql query und eine sql-injection ist möglich.

[Capfly]

Naja, durch mysql_real_escape_string() wird das zu einem \'
und ID ist dann keine Zahl, ein Fehler wird gemeldet, mehr passiert da eigtl auch nicht

[Amargedd0n]

Aber eien SQL-Injection ist doch möglich oder? :S