Ich habe nicht vor, mir das Ganze anzusehen, aber grob gesagt: Wenn Du Passwörter hashst, bevor Du sie in die Datenbank schreibst, und beim Einloggen auch wieder Hashes vergleichst, dann kannst Du schon mal nicht mehr viel falsch machen.

Nein, weil:
1) die mysql library ist deprecated, nutz MySQLi oder PDO
2) google mal "SQL injection"
3) kein Md5. Nutz password_hash
4) wenn kein PHP 5.5 stattdessen github.com/ircmaxell/password_compat oder so
5) Zeile 3-5 sorgt für einen Fehler, wegen Zeile 20/24 (header())
6) keine Fehlerprüfung in Zeile 17, ob der User überhaupt existiert
...

Edit: Punkt 5) stimmt eventuell nicht, weil der PHP-Parser ein newline nach dem schließenden Tag ignoriert. Solche Konstrukte sollte man dennoch vermeiden.

Besser als Klartext, aber noch nicht gut genug.
In erster Linie ist MD5 veraltet und sollte für solche Sachen nicht mehr verwendet werden. Eine Alternative wäre SHA256, aber siehe unten.
Und dazu kommt, dass hier der Salt fehlt. Also ein Angreifer, der die Tabelle mit den Hashes hat, kann einfach jeden Hash durch eine RainbowTable jagen und dann kommt er auf das Passwort, das diesen Hash erzeugt hat. Und dazu kommt auch, dass man sofort sieht, welche Benutzer das gleiche Passwort haben, da der Hash gleich ist.
Es gibt in neueren PHP-Versionen diese beiden Funktionen:
php.net/manual/de/function.password-hash.php
php.net/manual/de/function.password-verify.php
Die erste erstellt aus einem Passwort einen String, wo alles nötige drin steht. Hash, Salt und Einstellungen. Und die zweite prüft, ob das angegebene Passwort mit dem angegebenen "Hash" zusammenstimmt.
Also der folgende Code ist eine Tautologie:

Und was von password_hash zurückkommt, speicherst Du in der Datenbank.


Das andere Problem ist SQL Injection: Was sind Injections? - Kleine Erklärung
Ich kann Deinem Server als UserName folgendes schicken: Blubb' OR 'a' = 'a
Dann steht in Deiner Abfrage das drin:

beim anmelden schreibt der benutzer in der regel den benutzername und das password in ein textfeld und du gibst das was er reinschreibt eins zu eins weiter
Beispiel wenn ich dann in das textfeld username " '; DROP TABLE login " dann wird die Tabelle gelöscht... natürlich lassen sich so auch schlimmere befehle wie "nur" das löschen injecten.

gegenmaßnahme ist, dass du die benutzereingabe escapest mit mysqli z.b. mysqli_real_escape_string

Wichtig, das Hashen muss auf Serverseite stattfinden! Ansonsten kann man Dir den Hash bereits auf dem Weg vom Client zum Server abgreifen und sich direkt damit einloggen, sodass man gar kein ursprüngliches Passwort mehr braucht. Um natürlich nicht im Klartext zu übertragen, solltest Du dann am Besten doppelt hashen. Aber auch das bietet keine 100%ige Sicherheit.
Die beste Lösung wäre somit ein eigentlich unabdingliches SSL-Zertifikat, um die Verbindung ganz zu verschlüsseln.

Und wie gesagt, MD5 nicht mehr verwenden, nimm gesalzenes SHA256.

Grüße

@Orion

Wie kann ich denn Injections verhindern?

Nennt sich Prepared Statements. mysql_real_escape_string sollte man nicht verwenden. Man schießt sich damit nur ins Knie, wenn man einen Fehler macht. Davon abgesehen: Wenn jemand in dieser Funktion eine Lücke findet, bist Du erst wieder am A.
Ich sehe auch gerade, dass Du die veralteten mysql_*-Funktionen verwendest. Die sind ebenfalls veraltet.
Verwende die mysqli_*-Funktionen (man beachte das i hinter mysql) oder noch besser gleich DBO.
Mit mysqli kann ich Dir prinzipiell zeigen, wie es aussehen sollte:
(Natürlich fehlen hier überall die Fehlerbehandlungen)
Also man ersetzt im Statement alles, was vom Code kommt, durch Fragezeichen.
Dann beim Aufruf von bind_param gibt man als ersten Parameter einen String an, wo jedes Zeichen angibt, welcher Typ für welches Fragezeichen verwendet wird und anschließend die Werte für die Fragezeichen.
"s" = Zeichenfolge (weil "String"), "i" = Ganzzahl (weil "Integer"), "d" = Gleitkommazahl (weil "Double").
Also im Beispiel oben wird Spalte1 mir dem Integer 123 verglichen und AndereSpalte mit einem komplexen String mit vielen Sonderzeichen.

Beachte, dass Du da ein bisschen was umschreiben musst, denn die Datenbank kann password_verify nicht für Dich ausführen!
Du musst den Benutzer mit dem eingegebenen Benutzernamen finden und dann in PHP das Passwort prüfen.

@Trade
Warum muss das hashen serverseitig geschehen? Ob ich per MITM den Hash oder das Kennwort abgreife ist egal. Es kann sogar Vorteilhaft sein, die Berechnung des Hash auf den Client vorzunehmen, weil dann das Kennwort bei einem MITM-Angriff nicht bekannt wird (welches der Benutzer vllt. noch woanders verwendet). Hat natürlich auch wieder Nachteile.

Trade schrieb:

Die beste Lösung wäre somit ein eigentlich unabdingliches SSL-Zertifikat, um die Verbindung ganz zu verschlüsseln.

Jein. Nur falls das jemand falsch versteht: Mit Passworthashing will man ein anderes Problem lösen als mit SSL. Natürlich sollte der Verbindung verschlüsselt werden, aber gehasht werden muss dennoch.

Trade schrieb:

Und wie gesagt, MD5 nicht mehr verwenden, nimm gesalzenes SHA256.

Ja und Nein. md5 und die SHA-Familie sind beide nicht für Passwörter konzipiert und daher ungeeignet. Der Grund ist: sie sind zu effizient.
Wenn man nichts anderes hat, ist sha2 natürlich md5 vorzuziehen (salt sollte selbstverständlich sein), aber wenn es um Passwörter geht ist man z.B. mit bcrypt deutlich besser bedient.
Und um Neulinge besser zu unterstützen, haben die PHP-Entwickler die weiter oben genannten password_*-Funktionen eingeführt, damit typischee Fehler (md5, kein salt, ...) langsam ausgemerzt werden.

3daycliff schrieb:

Ob ich per MITM den Hash oder das Kennwort abgreife ist egal

Wenn Du den Hash abgreifst und dieser auch so in der Datenbank steht, kann ich mich damit einloggen, ohne das Passwort zu haben. Ich kann den Hash also direkt verwenden. Was ich meinte, war, dass der Hash selbst für die Datenbank dann auf dem Server samt Salt etc. generiert und dann abgespeichert wird.

3daycliff schrieb:

Es kann sogar Vorteilhaft sein, die Berechnung des Hash auf den Client vorzunehmen, weil dann das Kennwort bei einem MITM-Angriff nicht bekannt wird (welches der Benutzer vllt. noch woanders verwendet)

Das meinte ich damit, dass es dann nicht im Klartext übertragen wird. Deswegen doppelt hashen. Aber besser wäre nat. eine verschlüsselte Verbindung.

3daycliff schrieb:

Natürlich sollte der Verbindung verschlüsselt werden, aber gehasht werden muss dennoch.

Ja, das wollte ich damit sagen. Natürlich beides, nicht nur SSL.

Bei den Hashes kann ich mich dann nur noch auf Deine Aussagen verlassen. Ich hielt immer SHA256 für ganz angemessen, aber dann übernehme ich das mal so, wenn dem so ist.

Grüße

Sorry wenn ich nochmal kram...
ich mach meine PHP Logins mit sha512 für Benutzername und für Kennwort
In der DB steht beispielsweise
|ID|NICKNAME|HASH_NICKNAME|HASH_PASSWORT
1 | samson | 9c482bbeb687ad47edb5ebf84668ced0a895a81054dd73ee41e27f865aceff394042f989ebea73395624a14c03d4b393b66de31496bdb36f57494d5a06fd5a5e | 7a7cfc99db9802272d1987e287926ac52642417bf2d68455e180412c226430d0321610e8740a46783b2b42802918d62d206d3b1b21dc600b6944ddecf5d9c256

Gepostet wird "im Klartext"
bevor es zur DB Anfrage kommt jagt man es durch den Hash...
Damit kommt nur noch "mist" bei raus und eine Injektion ist weniger möglich.
Wenn man nun noch einen Server hat der ein SSL Zertifikat hat, wunderbar.

Aber ab hier ist das ja nicht alles... Probleme gibts ja nicht nur beim Login,...
Was ist, wenn der Angreifer einen Account für den Bereich hat?
Sobald er auf ein "Suchformular" kommt / beispielsweise Forum durchsuchen \
ist schon wieder mysqli_real_escape_string notwendig und sicher ist sicher... besser noch addslashes...
Damit werden die unschönen ' rausgestrichen.

Ich hoffe ich konnte ein wenig helfen, wie ich da mache... Sollte meins auch nicht so optimal sein, lasse ich mich gerne eines besseren belehren
Grüße samson

samson schrieb:

Gepostet wird "im Klartext"

Wie erwähnt, doppelt hashen + SSL-Zertifikat und dann passt das.

samson schrieb:

Damit kommt nur noch "mist" bei raus und eine Injektion ist weniger möglich.

Jeder Benutzerinput muss escaped werden, egal, was es ist.

samson schrieb:

mysqli_real_escape_string

Oder entsprechend gleich Prepared Statements verwenden, was wohl sowieso die bessere Variante ist.

Grüße

@Trade da geb ich dir recht

Trade schrieb:

Oder entsprechend gleich Prepared Statements verwenden, was wohl sowieso die bessere Variante ist.

Selbst, wenn Du es abfragst, ist es immer eine Pflicht den Input zu escapen, egal, was es ist.

Grüße

Trade schrieb:

ist es immer eine Pflicht den Input zu escapen, egal, was es ist.

Das stimmt so nicht ganz. Klar kann man wirklich jede einzige Angabe escapen, aber das ist nicht unbedingt sinnvoll.

Wenn ich die Benutzerangabe vorher hashe oder bspw. in Integer/Float/DateTime konvertiere, kann ich mir sicher sein, dass von einem eventuellen Angriff nichts mehr übrig ist (außer vllt eine Exception oder eine sinnlose Anfrage). Besonderes Augenmerk liegt auf dem "vorher", denn das ganze muss auf PHP-Ebene geschehen. Wenn du die SQL-Funktionen zum Hashen oder konvertieren benutzt, bist du natürlich nach wie vor angreifbar.

Also wenn man weiß, was man tut, braucht man nicht alles per se escapen.

Oder man escapet einfach immer, schadet ja nicht. Die meisten ORM-Layer arbeiten ja auch so (weil die gar nicht wissen, was es für Daten sind).

Jo, ich würd's so trotzdem immer machen, einfach zur Sicherheit. Zwar ist es in der Tat öfters mal der Fall, dass eine Injection so nicht einfach möglich ist, da man durch gewisse Konvertierungen, Bedingungen, whatever.. das Ganze schon insofern validiert hat, dass der String nicht manipuliert wurde, aber dennoch ist man so einfach in jedem Fall auf der sicheren Seite.
Und wie gesagt, schadet ja nicht und ist mit Prepared Statements entsprechend auch gleich nebenzu gemacht und kein großer Aufwand.

Grüße