nicht angemeldet
Hallo.
MudGuard hat ja schon einiges zum Code geschrieben - dem kann ich soweit beipflichten, bis auf den zweiten Punkt.
$name = $_POST['name'];
Halte ich für durchaus legitim, wenn die Daten danach sofort validiert werden. Anscheinend tust du das ja. Aber damit sind wir schon bei
include("komm_vali.php");
1. Die Qualität deines Skriptes hängt sehr stark davon ab, was in diesem eingefügten File tatsächlich getan wird. Vielleicht kannst du den Inhalt hier mal posten. Auf alle Fälle solltest du es mit require() einbinden, da du sicher nicht möchtest, dass das Skript weiterläuft, wenn die Daten nicht validiert werden konnten. Das ist gerade darum notwendig, weil die Validierung anscheinend false zurückliefert, wenn die Daten nicht zu beanstanden sind.
if (!$error\_msg\_1 &&!$error\_msg\_2 &&!$error\_msg\_3 &&!$error\_msg\_4 &&!$error\_msg\_5 &&!$error\_msg\_6 &&!$error\_msg\_7) { writeComment(...
Um ganz sicher zu gehen solltest du mit === testen ob die Variablen tatsächlich false sind und nicht etwa '0'.
2. Ich kann nur nochmal betonen, was MudGuard schrieb: nutze mysql_real_escape_string()! Wenn jemand einen Namen mit Apostroph besitzt mag die Fehlermeldung zwar ärgerlich sein, der richtige Schaden entsteht aber, wenn jemand gezielt falschen mysql-Code eingibt (SQL-Injektion). Solche Fehler sollte man sich garnicht erst angewöhnen! Den Claim "All userdata is evil." solltest du beim Programmieren nicht nur im Hinterkopf haben.
mysql_query ("INSERT INTO comments (id, autor, datum, uhrzeit, website, email, kommentar)
3. Hier würde ich für Datum und Uhrzeit den MySQL-Typ DATETIME verwenden. Das hat auch den Vorteil, dass du hier
$query = "SELECT * FROM comments ";
die Kommentare nach Datum sortiert ausgeben kannst.
$ausgabe_Autor = $row_out['autor'];
4. Ein weiterer großer Sicherheitsfehler! Da muss auf alle Fälle irgendwo ein htmlspecialchars() rein, sonst gebe ich dir als böser Mensch, der ich nun mal bin, soetwas wie
<script>alert('Ich bin böse.');</script>
in dein Formular ein, oder zeige damit jedem deiner Besucher meine Layer-Ads an, lese ihre Cookies aus, setze mein eigenes Cookie und tue noch so einige andere böse Sachen. Mehr Informationen zum Cross-Site-Scripting gibt es hier.
Je nachdem welche Codierung du auf deinen Seite verwendest, kann es auch sinnvoll sein htmlentities() zu verwenden. Außerdem solltest du dich im Formular noch mit http://de.selfhtml.org/html/formulare/definieren.htm#zeichenkodierung@title=accept-charset auf einen Zeichensatz festlegen (am besten den, den du auch für den Rest der Seite verwendest).
$_SERVER['PHP_SELF'];
5. Das Statement tut nichts. Sicher hast du das echo davor vergessen, aber genaugenommen ist das überflüssig. Wenn action leer ist wird automatisch auf die gleiche Seite verwiesen.
if($ausgabe_Website == "http://")
6. Nicht gut. Erstens würde ich keinen sinnlosen String speichern und zweitens spuckt die das else alles aus, was nicht 'http://' ist. Aber hier sind wir auch wieder bei der Blackbox namens 'include("komm_vali.php")'. Auf alle Fälle solltest du einen besseren Test wählen oder sichergehen, dass sonst nur zulässige URLs in der Datenbank gespeichert werden.
Soweit alles, das mir aufgefallen ist.
Christian
MudGuard
Auge