nicht angemeldet
Sicherheitslücken?
Da ich ein ziemlicher Neuling bin und mir php und Mysql bislang nur mit Tutorials beigebracht habe, hätte ich gerne eine Kontrolle bezüglich Sicherheitslücken.
Ich hoffe jemand von euch hat die Zeit, das mal zu überfliegen.
Die Seite funktioniert in diesem Zustand, die Frage ist nur ob sich da Sicherheitslücken auftun.
Dass das Coding wahrscheinlich schöner möglich ist ist mir klar, aber solange es keine Schwachstelle bietet reicht mir das.
Ich hab hier mal die Index.php und ein Beispiel für die Administration, der Rest würde dann im selben Schema aufgebaut werden, wenn ihr nicht noch etwas findet.
Die index.php:
http://nopaste.com/p/a406cSvFR
Die Dateien news.php und news_b.php im durch htaccess geschützten Ordner "admin":
news.php:
http://nopaste.com/p/axHeZJmAu
news_b.php:
http://nopaste.com/p/aXkIRajpn
Wäre nett wenn mir jemand da helfen könnte :)
-
echo $begrüßung;
Da ich ein ziemlicher Neuling bin und mir php und Mysql bislang nur mit Tutorials beigebracht habe, hätte ich gerne eine Kontrolle bezüglich Sicherheitslücken.
Ich hoffe jemand von euch hat die Zeit, das mal zu überfliegen.Ich fange mal mit news_b.php an. Lücken sind mir beim Fliegen nicht aufgefallen, aber verbesserungswürdige Dinge.
include("../db_connect.php");
Die Klammern sind überflüssig, include ist keine Funktion.
$datum = $_POST["datum"];
...Immer wieder gern gemacht und nicht nur unnötig sondern auch bedenklich. Du prüfst zum einen nicht, ob $_POST['datum'] überhaupt existiert, sondern greifst einfach drauf zu. Dann kopierst du den Inhalt ohne Not in eine harmlos aussehende Variable um. Man muss sich nun merken, dass dieser Wert eine Benutzereingabe ist. Bei einem $_POST['foo'] sieht man das hingegen ohne weiteres.
$datum = mysql_real_escape_string($datum);
...Diese Aktion ist zwar richtig, aber ihre Stelle kann besser gewählt werden. Auch hier muss man sich wieder merken, dass $datum (und die anderen Werte) bereits SQL-gerecht aufbereitet sind. Was ich nicht sehe ist eine Gegenbehandlung zu den Magic Quotes. Sind die bei dir generell deaktivert? Ja: gut. Nein: Disabling Magic Quotes.
Eine passendere Stelle ist genau beim Zusammenbau des Statements. Dann kann es auch nicht passieren, dass du dieses erweiterst und oben vergisst, eine Zeile hinzuzufügen. Statt:
$editieren = "UPDATE d_news SET datum = '$datum', newstext = '$newstext' WHERE id = '$id'";
empfehle ich:
$editieren = sprintf("UPDATE d_news SET datum = '%s', newstext = '%s' WHERE id = '%s'",
mysql_real_escape_string($datum),
mysql_real_escape_string($newstext),
mysql_real_escape_string($id));Du schwindelst,
$edit = mysql_query($editieren);
$msg = "Die Nachricht wurde erfolgreich editiert.";denn du hast hier gar nicht geprüft, ob das Statement erfolgreich ausgeführt wurde.
Weiter in index.php:
setcookie(admin_einblenden, yes, time()+(3600*24*100));
Was sind denn admin_einblenden und yes für Dinge? Konstanten, die in db_connect.php definieret wurden? Wenn nicht, dann fehlt die Kennzeichnung als String. Stelle bitte das error_reporting auf E_ALL (und display_errors auf on), dann siehst du, warum diese Stelle trotzdem "funktioniert" hat.
Apropos db_connect.php. Hast du dort nach dem Öffnen der Verbindung die in deinen Scripten verwendete Zeichenkodierung explizit mit dem MySQL-Server ausgehandelt? Sonst ist es Zufall, wenn Zeichen jenseits von ASCII (also auch Umlaute) richtig behandelt werden. Hast du dir überhaupt Gedanken um die Zeichenkodierung gemacht?
$starttext_abfrage = "SELECT * FROM d_starttext WHERE id = 1";
$text = mysql_query($starttext_abfrage);
while($row = mysql_fetch_object($text))Hier prüfst du ebenfalls nicht, ob das Statement erfolgreich ausgeführt werden konnte, sondern machst einfach weiter. Im Fehlerfall liefert mysql_query() ein false zurück, was aber kein gültiges Argument für die Fetch-Funktionen ist.
<?php echo $titel; ?>
Preisfrage: Ist $titel HTML-gerecht ausgegeben worden oder nicht?
If($choice == "create")
{
echo $create;
}
elseif($choice == "edit")
{
echo $edit;
}
elseif($choice == "kill")
{
echo $kill;
}
else
{
echo "Fehlerhafte Anweisung!";
}Hier plädiere ich für ein switch, weil das übersichtlicher zeigt, was gewünscht ist
Überfliegen kann natürlich keinen gewissenhaften Test ersetzen. Den solltest du selbst so ausführlich wie möglich machen. Prüfe dabei auch mit ungewöhnlichen Eingabewerten. Wenn du Zahlen erwartest, gibt Buchstaben ein. Sonderzeichen (insbesondere ', ", \ sowie < und > (am besten als HTML-Tag, z.B. <hr>)) sollten in jedem Eingabefeld zumindest exemplarisch getestet werden. Lass solche Werte in die Datenbank eintragen und sieh dir an, ob die Ausgabe unverfälscht erfolgt. Versuch dein Script zu komprommitieren und zum Fehlverhalten zu bringen. Wenn du das nicht machst, und dabei Schwachstellen entdeckst, macht es jemand anderes.
Schau dir dann das Ganze auch aus der Sicht des Anwenders an. Sind alle Ausgaben (insbesondere Fehlermeldungen) verständlich und auch im Fehlerfall noch zielweisend und nicht nur Hinweise an den Programmierer?
echo "$verabschiedung $name";
-
Puh, erst einmal Danke für deine Antwort. Ich versuch das ganze mal der Reihe nach abzuhandeln.
include("../db_connect.php");
Die Klammern sind überflüssig, include ist keine Funktion.
Ok, ist überall verbessert.
$datum = $_POST["datum"];
...Immer wieder gern gemacht und nicht nur unnötig sondern auch bedenklich. Du prüfst zum einen nicht, ob $_POST['datum'] überhaupt existiert, sondern greifst einfach drauf zu. Dann kopierst du den Inhalt ohne Not in eine harmlos aussehende Variable um. Man muss sich nun merken, dass dieser Wert eine Benutzereingabe ist. Bei einem $_POST['foo'] sieht man das hingegen ohne weiteres.
$datum = mysql_real_escape_string($datum);
...Diese Aktion ist zwar richtig, aber ihre Stelle kann besser gewählt werden. Auch hier muss man sich wieder merken, dass $datum (und die anderen Werte) bereits SQL-gerecht aufbereitet sind. Was ich nicht sehe ist eine Gegenbehandlung zu den Magic Quotes. Sind die bei dir generell deaktivert? Ja: gut. Nein: Disabling Magic Quotes.
Eine passendere Stelle ist genau beim Zusammenbau des Statements. Dann kann es auch nicht passieren, dass du dieses erweiterst und oben vergisst, eine Zeile hinzuzufügen. Statt:
$editieren = "UPDATE d_news SET datum = '$datum', newstext = '$newstext' WHERE id = '$id'";
empfehle ich:
$editieren = sprintf("UPDATE d_news SET datum = '%s', newstext = '%s' WHERE id = '%s'",
mysql_real_escape_string($datum),
mysql_real_escape_string($newstext),
mysql_real_escape_string($id));Ok, ich hab die real_escape_strings an die richtige Stelle gesetzt und überall statt der Variablen direkt das $_POST["xyz"] eingesetzt. Wie ich prüfe ob es das gibt weiß ich allerdings nicht, da bräuchte ich noch einen kleinen tipp.
Die Magic Quotes sind laut phpinfo ausgeschaltet, stellen also keine "Gefahr" dar.Du schwindelst,
$edit = mysql_query($editieren);
$msg = "Die Nachricht wurde erfolgreich editiert.";denn du hast hier gar nicht geprüft, ob das Statement erfolgreich ausgeführt wurde.
Auch hier wieder: Wie mache ich das am besten? In meinem Lieblingstutorial scheint dieser Teil zu fehlen :P
Weiter in index.php:
setcookie(admin_einblenden, yes, time()+(3600*24*100));
Was sind denn admin_einblenden und yes für Dinge? Konstanten, die in db_connect.php definieret wurden? Wenn nicht, dann fehlt die Kennzeichnung als String. Stelle bitte das error_reporting auf E_ALL (und display_errors auf on), dann siehst du, warum diese Stelle trotzdem "funktioniert" hat.
Er meldet mir ein
"Notice: Undefined index: admin in /home/www/webx/html/xyz/index.php on line 10"
Von den Angaben im Cookie ist nichts irgendwie definiert, der Teil soll dazu dienen die Symbole zur Administration nur auf Wunsch per Cookie einzublenden. Sieht für den normalen Benutzer ohne eben schöner aus.
Wenn ich das richtig verstanden habe ist admin_einblenden der Name des Cookies und yes der Teil, mit dem ich dann prüfe ob die If Anweisung nun wahr ist oder nicht.Dadurch ist auch klar, dass $admin = $_GET['admin']; nur funktioniert wenn auch jemand auf den entsprechenden Link ganz unten auf der Seite geklickt hat.
Apropos db_connect.php. Hast du dort nach dem Öffnen der Verbindung die in deinen Scripten verwendete Zeichenkodierung explizit mit dem MySQL-Server ausgehandelt? Sonst ist es Zufall, wenn Zeichen jenseits von ASCII (also auch Umlaute) richtig behandelt werden. Hast du dir überhaupt Gedanken um die Zeichenkodierung gemacht?
Zu meiner Schande nein. Was sollte ich dort am besten schreiben?
In db_connect sind nur die Befehle "mysql_connect" und "mysql_select"$starttext_abfrage = "SELECT * FROM d_starttext WHERE id = 1";
$text = mysql_query($starttext_abfrage);
while($row = mysql_fetch_object($text))Hier prüfst du ebenfalls nicht, ob das Statement erfolgreich ausgeführt werden konnte, sondern machst einfach weiter. Im Fehlerfall liefert mysql_query() ein false zurück, was aber kein gültiges Argument für die Fetch-Funktionen ist.
Hier das selbe wie oben, ich bräuchte diesbezüglich etwas Nachhilfe.
<?php echo $titel; ?>
Preisfrage: Ist $titel HTML-gerecht ausgegeben worden oder nicht?
Ich hab es mit mysql_real_escape_string in die Datenbank eingetragen wenn du das meinst.
If($choice == "create")
{
echo $create;
}
elseif($choice == "edit")
{
echo $edit;
}
elseif($choice == "kill")
{
echo $kill;
}
else
{
echo "Fehlerhafte Anweisung!";
}Hier plädiere ich für ein switch, weil das übersichtlicher zeigt, was gewünscht ist
"Switch"?
Ich hab alle Eingaben mal auf verschiedene Weise durchgemacht, bislang konnte ich keine Fehler verursachen.
Schonmal danke für deine kompetente Hilfe, ich hoffe du kannst auch die letzten Unklarheiten noch klären.
Gruß Tesa
-
echo $begrüßung;
[...] überall statt der Variablen direkt das $_POST["xyz"] eingesetzt. Wie ich prüfe ob es das gibt weiß ich allerdings nicht, da bräuchte ich noch einen kleinen tipp.
Die Funktion isset() prüft auf das Vorhandensein von Variablen. Da Arrayelemente auch nur Variablen sind kann man sie auch darauf anwenden:
if (isset($_POST['xyz'])) ...
bzw.
if (! isset(...)) setze Defaultwert.setcookie(admin_einblenden, yes, time()+(3600*24*100));
Was sind denn admin_einblenden und yes für Dinge?
Wenn ich das richtig verstanden habe ist admin_einblenden der Name des Cookies und yes der Teil, mit dem ich dann prüfe ob die If Anweisung nun wahr ist oder nicht.Das sind doch sicher Strings. Notiere sie als solche in Anführungszeichen. Ohne Anführungszeichen sind es Konstanten-Bezeichner, die PHP aber beim Nichtfinden einer mit diesem Namen definierten Konstante als Strings ansieht.
Apropos db_connect.php. Hast du dort nach dem Öffnen der Verbindung die in deinen Scripten verwendete Zeichenkodierung explizit mit dem MySQL-Server ausgehandelt? Sonst ist es Zufall, wenn Zeichen jenseits von ASCII (also auch Umlaute) richtig behandelt werden. Hast du dir überhaupt Gedanken um die Zeichenkodierung gemacht?
Zu meiner Schande nein. Was sollte ich dort am besten schreiben?Zum Einstieg in das Thema empfehle ich aus dem Themenbereich http://de.selfhtml.org/inter/index.htm@title=Internationalisierung mindestens den Artikel <http://de.selfhtml.org/inter/sprache.htm@title=Computer und geschriebene Sprache>. Auch die nächsten beiden können nicht schaden. Du müsstest dich dann entscheiden, ob du ISO-8859-1/Latin1 oder UTF-8 verwenden möchtest. ISO-8859-1 ist hierzulande (nach meinem Gefühl) noch am weitesten verbreitet. UTF-8 mit seinen Vorteilen gewinnt zwar Land, stellt aber Anfänger und teilweise auch die Systeme (z.B. PHP kleiner als Version 6) gelegentlich vor Probleme.
Jedem am Datenverarbeitungsprozess beteiligten System muss klar sein, welche Kodierung die zu verarbeitenden Daten haben. Das muss beziehungsweise sollte man dem System mitteilen, wenn man sich nicht auf die gegebenenfalls unterschiedlichen Default-Einstellungen verlassen möchte. Für HTTP als charset-Angabe im Content-Type-Header, für HTML im Meta-Element Content-Type (HTTP hat Vorrang vor der HTML-Angabe). Selbstverständlich müssen die Daten dann auch in dieser Kodierung vorliegen. Das setzt als Folge auch einen Editor voraus, der in der gewünschten Kodierung speichern kann. (UTF-8 bitte ohne BOM speichern.)
Um dem MySQL-Server die auf der Verbindung zwischen ihm und PHP zu verwendende Kodierung mitzuteilen gibt es die Funktion mysql_set_charset(). Falls diese noch nicht vorhanden ist, geht auch ein nach dem Verbindungsaufbau abgesetztes Statement SET NAMES latin1 bzw. utf8.
Auch die Kodierungsangabe der einzelnen Felder sollte angemessen eingestellt sein. Angaben der Tabelle oder der Datenbank zählen nur als Default-Wert für neu angelegte Felder oder Tabellen.Das war auf die Schnelle nur das Wichtigste. Bei Bedarf bitte auch das hiesige Archiv befragen.
In db_connect sind nur die Befehle "mysql_connect" und "mysql_select"
$starttext_abfrage = "SELECT * FROM d_starttext WHERE id = 1";
$text = mysql_query($starttext_abfrage);
while($row = mysql_fetch_object($text))Hier prüfst du ebenfalls nicht, ob das Statement erfolgreich ausgeführt werden konnte, sondern machst einfach weiter. Im Fehlerfall liefert mysql_query() ein false zurück, was aber kein gültiges Argument für die Fetch-Funktionen ist.
Hier das selbe wie oben, ich bräuchte diesbezüglich etwas Nachhilfe.Werte das Ergebnis der mysql_*-Funktionen aus und reagiere angemessen. Informiere dich im Handbuch, wann welches Ergebnis zurückgegeben wird. Oft sieht man
mysql_...(...) or die(fehlermeldung);
Das ist schnell notiert aber kein guter Stil. Wegen Datenbankfehler muss man keinen Script-Selbstmord begehen. Besser ist:
if ($connection = mysql_connect(...)) {
Tu was mit der Connection.
} else {
Erzähl dem Anwender, dass sein Anliegen grad nicht ausgeführt werden konnte, aber keine technischen Details.
Technische Details (z.B. die Ausgabe von mysql_error()) braucht nur der Administrator. Informiere diesen, z.B. per Mail.
Gib Alternativen an, die dem Anwender helfen, sein Ziel dennoch zu erreichen.
}Nach diesem Prinzip müsstest du auch mit den Querys verfahren.
Da du die Connection in einer Include-Datei aufbaust, lässt sich dieses if-Konstrukt nicht direkt wie oben notiert anwenden. Statt dessen müsstest du im Haupt-Script prüfen, ob die Verbindung eine ist oder nicht. Es reicht dabei, sie per
if ($connection)
zu testen.<?php echo $titel; ?>
Preisfrage: Ist $titel HTML-gerecht ausgegeben worden oder nicht?
Ich hab es mit mysql_real_escape_string in die Datenbank eingetragen wenn du das meinst.Nein, falscher Kontext. Die Behandlung mit mysql_real_escape_string() ist nicht für die Speicherung in der Datenbank da sondern für die Notation im SQL-Statement. Nur auf dem Weg zum SQL-Server müssen die Daten in ein SQL-Statement eingebettet werden und dabei muss unterschieden werden können ob Zeichen (wie ', " und ) zu den Daten gehören oder Syntax-Bestandteil (z.B. Stringbegrenzungszeichen) sind. In der Datenbank kommen die Zeichen in ihrer unbehandelten Form zu liegen und bei einer Abfrage auch so wieder zum Client zurück. Wenn du sie nun in einen HTML-Kontext bringst, musst du sie nun für den HTML-Kontext behandeln. Das hast du an anderen Stellen ja schon mit htmlspecialchars() praktiziert.
If($choice == "create")
elseif($choice == "edit")
elseif($choice == "kill")
else
Hier plädiere ich für ein switch, weil das übersichtlicher zeigt, was gewünscht ist
"Switch"?Ich hab alle Eingaben mal auf verschiedene Weise durchgemacht, bislang konnte ich keine Fehler verursachen.
Schon mal sehr gut. Bedenke aber, dass Sicherheit kein Ruhekissen ist.
Schonmal danke für deine kompetente Hilfe, ich hoffe du kannst auch die letzten Unklarheiten noch klären.
Falls noch was übrig geblieben ist oder neue Fragen aufgetaucht sind, frag einfach. Beantworten muss es aber jemand anderes. Ich bin dann mal weg.
echo "$verabschiedung $name";
-
-