nicht angemeldet
Ist dieser code sicher (SQL Injection)?
Guten Morgen zusammen,
folgender Code funktioniert so wie ich mir das vorstelle, auch die Nutzer der Seite sind zufrieden, aber ...
ich bin mir nicht sicher, ob der Code ausreichend sicher ist: Stichwort SQL Injection. Die Daten selbst sind nicht hochsensibel. Mir geht es hauptsächlich darum, daß niemand an die DB kommt.
Es handelt sich insgesamt um einen geschützten Downloadbereich, in dem zwischen mir und ca. 5 Kunden, Dateien ausgetauscht werden (bislang ohne SSI- Verschlüsselung), Einbahnstraße: ich lade hoch per FTP, sie laden runter per Link. Es gibt von mir festgelegte Benutzernamen und Passwörter, die vom Kunden nicht geändert werden können.
Mit Benutzername und Passwort loggt man sich in den Downloadbereich ein, erhält dort eine Liste mir höchstens 10 downloadbaren Dateien als Link. Das Verzeichnis, in dem die Dateien liegen ist noch einmal mit dem Paßwortschutz des Providers geschützt (.htaccess?)
Hier der Code der fraglichen Seite, $pass und $uname kommen von der Login-Seite per POST
<pre>
... irgendetwas html ...
<?php include ("namen.php");
... irgendetwas html ...
<body>
<div id="login">
<?php
$pass = md5($pass);
if("" != $uname ){
include("myconnect_u.php");
$query="SELECT * FROM tblbenutzer WHERE benutzer ='".mysql_escape_string($uname)."'";
$result=mysql_query($query);
$row=mysql_fetch_object($result);
if(!$row){
$fehler="falsch";
include ("myindexfalsch.php");
} else{
if(md5($row->passwort)==$pass){
include ("myfilelist_u.php");
} else {
$fehler="falsch";
include ("myindexfalsch.php");
}
}
} else {
$fehler="ohne";
include ("myindexfalsch.php");
}
?>
</pre>
Vielen Dank schon mal für Eure Anmerkungen
-
Sup!
mysql_escape_string ist deprecated.
Gruesse,
Bio
-
$query="SELECT * FROM tblbenutzer WHERE benutzer ='".mysql_escape_string($uname)."'";
Hi,
schau dir mal das an, das ist besser dafür geeignet.
-
echo $begrüßung;
ich bin mir nicht sicher, ob der Code ausreichend sicher ist: Stichwort SQL Injection.
Wenn du so fragst, scheint mir, dass du das Prinzip nicht verstanden hast. Der Grundsatz lautet: Wann immer du Daten in einen anderen Kontext bringst, müssen sie kontextgerecht behandelt werden. In deinem Fall bringst du Daten in den Kontext eines SQL-Statements. Damit der Empfänger des Statements, der MySQL-Server, weiß, was Daten und was Befehlsbestandteil ist, braucht er eine Kennzeichnung der Daten. Vorgeschrieben ist, dass diese mit einfachen oder doppelten Anführungszeichen einzurahmen sind. Was passiert nun, wenn ein solches Zeichen in den Daten vorkommt? Es beendet die Zeichenfolge. Alles dahinter wird wieder als Befehlsbestandteil angesehen. Deshalb müssen die Anführungszeichen (und noch ein paar weitere) maskiert werden. Ihnen wird im Falle MySQLs ein Backslash vorangestellt, und damit verlieren sie ihre Bedeutung als Stringbegrenzer. Die Aufgabe der Maskierung übernimmt mysql_real_escape_string(). So wie du sie angewendet hast ist das schon mal richtig, aber es geht übersichtlicher.
$query = sprintf("SELECT * FROM tblbenutzer WHERE benutzer ='%s'", mysql_real_escape_string($uname));
Durch sprintf() und den Platzhalter %s spart man sich das String-Rein-Raus-und-Verknüpfen. Hier noch ein Beispiel für zwei Parameter:
$query = sprintf("SELECT * FROM tblbenutzer WHERE benutzer ='%s' AND password='%s'",
mysql_real_escape_string($uname),
mysql_real_escape_string($password));Eine weitere Möglichkeit wären Prepared Statements. Die arbeiten im Prinzip wie das sprintf() mit Platzhaltern, übertragen aber das Statement und die Daten auf getrenntem Weg. Dabei spart man sich auch gleich noch das Maskieren, weil die Daten ja nicht mehr in den Text eines Statements eingebettet werden müssen. P.S. lassen sich aber nur mit der moderneren mysqli-Extension von PHP5 nutzen.
Außerdem fehlt bei dir komplett das Reagieren auf Fehlerzustände. Die mysql-Funktionen geben einen solchen über ihr Funktionsergebnis bekannt. Es ist dann False statt einer Ressourcenkennung. False ist aber kein gültiges Argument für die nachfolgenden mysql-Funktionen, was dann zu PHP-Fehlermeldung führt. Wenn du deine Anwendung robuster gestalten willst, bau eine Fehlerbehandlung ein. Ein "... or die(mysql_error);" ist zwar schnell eingefügt aber keine Fehlerbehandlung. Der Anwender, der diese Meldung zu sehen bekommt, kann damit nichts anfangen (und wenn er es kann, soll er es nicht). Besser ist, das Script auch im Fehlerfall geordnet zu beenden, den Administrator geeignet zu benachrichtigen und dem Anwender eine Alternative aufzuzeigen, wie er dennoch sein Ziel erreichen kann.
echo "$verabschiedung $name";
-
Hi,
vielen Dank für Deine ausführliche Erläuterungen, die mir das Ganze tatsächlich etwas klarer gemacht haben.
$query = sprintf("SELECT * FROM tblbenutzer WHERE benutzer ='%s'", mysql_real_escape_string($uname));
So hatte ich das nach der ersten anregung auch geschrieben und mir dann das Beispiel von http://www.php.net/manual/de/function.mysql-real-escape-string.php genommen und angepaßt.
Außerdem fehlt bei dir komplett das Reagieren auf Fehlerzustände ... Fehlerfall geordnet zu beenden, den Administrator geeignet zu benachrichtigen ...
Da hast Du recht. Ich vertraue auf meinen Provider, daß die DB durchgängig läuft.
Durch den Beispielcode sind noch mehrere Fehler abgefangen. Der Administrator wird übers Telefon benachrichtigt! wir sind noch ganz nah bei unseren 5 Anwendern.Aber ich halte solche Informationen immer im Hinterkopf, denn auch diese Anwendung wird irgendwann komplexer und anonymer werden. Dann spätestens - vielleicht auch früher, wenn ich zum üben Zeit habe - werde ich Deine Anregung beherzigen.
Danke für die ausführliche Antwort
-
-
Danke für Eure Hilfe