Sicherheitslücken

[DJ Nuno]

Hallo,

vorab sorry für den langen text!
So, als ich soeben an einem Spiel bastelte wo es darum geht webseiten virtuell zu hacken etc. im zusammenhang mit Cross Site Scripting fiel mir meine eigene seite ein.

Ich also mal meine scripte angeguckt, und auf der Login page als benutzer name "> <hr /> eingegeben. Und da erscheint über die Loginpage eine Horiziontale Linie, htmlspecialchars vergessen.

Im Moment werden die POST Daten ungefiltert in den if anweissungen Verrarbeitet, für einträge in die datenbank erfolgt ständig vorher ein htmlspecialchars bevor die daten im SQL Befehl landen.
Eine Weitere lücke habe ich ja nun eben gefunden, indem ich aufpassen muss überall ein htmlspecialchars zu haben wenn POST Daten ausgegeben werden.

Zu Meinem Loginscript.
Mein Login Script setzt folgende Cookies:

setcookie("eingelogt", "1", time()+32248800);
setcookie("userid", $row['id'], time()+32248800);
setcookie("pass", $row['pass'], time()+32248800);
$_SESSION['nick'] = $row['nick'];
$_SESSION['userid'] = $row['id'];
$_SESSION['rank'] = $row['rank'];
$_SESSION['email'] = $row['email'];
$_SESSION['eingelogt'] = true;
session_regenerate_id(); 

Wenn die daten gelesen werden, bzw. bei jedem neuen Seiten aufruf passiert folgendes in der Index.php:

    if($_COOKIE['eingelogt'] == "1") {
        $db = mysql_connect('localhost', '***', '***') or die("MySQL Connect Error!");
        mysql_select_db('***', $db) or die("Database Select Error!");
        $sql = "SELECT * FROM reg_user WHERE id=".htmlspecialchars($_COOKIE['userid']);
        $result = mysql_query($sql) or die(mysql_error());
        $num = mysql_numrows($result);
        
        $row = mysql_fetch_array($result);
        if ($_COOKIE['pass'] == $row['pass']) {
            setcookie("eingelogt", "1", time()+32248800);
            setcookie("userid", $row['id'], time()+32248800);
            setcookie("pass", $row['pass'], time()+32248800);
            $_SESSION['nick'] = $row['nick'];
            $_SESSION['userid'] = $row['id'];
            $_SESSION['rank'] = $row['rank'];
            $_SESSION['email'] = $row['email'];
            $_SESSION['eingelogt'] = true;
            session_regenerate_id();
        } else {
            $_SESSION['nick'] = "";
            $_SESSION['id'] = "";
            $_SESSION['rank'] = "";
            $_SESSION['email'] = "";
            $_SESSION['eingelogt'] = false;
            setcookie("eingelogt", "0", time());
            setcookie("userid", "0", time());
            setcookie("pass", "0", time());
            header("Location: ".$_SERVER['PHP_SELF']."?link=cerror");
        }
    } 

Also meine frage:

Worauf muss ich noch achten?
Was ist noch wichtig?
Verbesserungsvorschläge?

[David]

Wozu denn _COOKIE und _SESSION? Wenn Du bei jeder Anfrage die Daten in _SESSION neu erstellen lässt, kannst Du Dir die Session eigentlich gleich klemmen.

[DJ Nuno]

hmm... stimmt wo du recht hast hast du recht... dann füge ich einfach folgene zeile hinzu damit es mehr sinn macht lol...

if (!isset($_SESSION['eingeloggt'])) { 

dann wird das script nur einmal ausgeführt... klingt komisch aber ich will die daten ungern in variablen speichern da später auch noch andere bereiche der homepage mit dem login zusammen hängen werden.

irgendwer noch welche tipps?

[David]

Ich komme immer noch nicht dahinter, warum Du ein Cookie setzt, wenn Du _SESSION verwendest.
Und $_SESSION['eingeloggt'] ist eine Variable. Deshalb verstehe ich

klingt komisch aber ich will die daten ungern in variablen speichern da später auch noch andere bereiche der homepage mit dem login zusammen hängen werden.

auch nicht.

[DJ Nuno]

weil eien sassion nicht ewig hält sondern nur für eine bestimmte zeit... jedenfalls kein jahr... die Sessions werden gesetzt weil auch ein cookie gefälscht werden kann...

Und der grund dafür das die Cookies und die Sessions bei jedem besuch neu gesetzt werden ist einfach der das die leute wenn sie meine homepage regelmässig besuchen dauerhaft eingeloggt bleiben... deshalb bei jedem besuch cookie neu setzten auf 1 jahr...

[David]

Mal abgesehen davon, dass ich es wenig brillant finde, Benutzernamen und Passwort im Klartext dauerhaft auf dem Rechner zu speichern, wofür ist

setcookie("eingelogt", "1", time()+32248800);

gut?

Wenn Du eine Umleitung per header("Location: ...") vornimmst, solltest Du vorher session_write_close() aufrufen, um auszuschließen, dass der Browser die neue Seite schon abruft bevor php die aktualisierten Sessiondaten speichern konnte.

$sql = "SELECT * FROM reg_user WHERE id=".htmlspecialchars($_COOKIE['userid']);

An dieser Stelle muss mysql_real_escape_strring() statt htmlspecialchars() stehen.
edit: ach, das reicht ja auch nicht, da der Wert nicht in Anführungszeichen gefasst ist. Da muss also eine Zahl stehen sonst knallt es.
"SELECT * FROM reg_user WHERE id=".intval($_COOKIE['userid']);

[DJ Nuno]

Mal abgesehen davon, dass ich es wenig brillant finde, Benutzernamen und Passwort im Klartext dauerhaft auf dem Rechner zu speichern, wofür ist setcookie("eingelogt", "1", time()+32248800); gut?

Wer sagt denn das das passwort im klartext da steht? was für einen grund sollte ich haben Passwörter in der Datenbank im Klartext zu speichern? das passwort ist in MD5 verschlüsselt und so bekommt es der user auch in den Cookie gesetzt.

Und die daten werden gespeichert und verglichen da sonst jemand mal eben fröhlich die User id im Cookie ändern könnte und somit als völlig anderer user eingeloggt wird. was nicht sinn und sache der geschichte ist.

Wenn Du eine Umleitung per header("Location: ...") vornimmst, solltest Du vorher session_write_close() aufrufen, um auszuschließen, dass der Browser die neue Seite schon abruft bevor php die aktualisierten Sessiondaten speichern konnte.

An dieser Stelle muss mysql_real_escape_strring() statt htmlspecialchars() stehen.

Danke!

[David]

Wer sagt denn das das passwort im klartext da steht? was für einen grund sollte ich haben Passwörter in der Datenbank im Klartext zu speichern? das passwort ist in MD5 verschlüsselt und so bekommt es der user auch in den Cookie gesetzt.

Macht keinen Unterschied. Du speicherst das Passwort genauso so, wie es an die Datenbank gesendet werden muss. Für die Datenbank ist es also Klartext, was Du da dauerhaft im Cookie abspeicherst.

ach so, und das setcookie("eingelogt", "1", time()+32248800); ist damit auch noch nicht erklärt.

[DJ Nuno]

Macht keinen Unterschied. Du speicherst das Passwort genauso so, wie es an die Datenbank gesendet werden muss. Für die Datenbank ist es also Klartext, was Du da dauerhaft im Cookie abspeicherst.

was würdest du stattdessen empfehlen?

ach so, und das setcookie("eingelogt", "1", time()+32248800); ist damit auch noch nicht erklärt.

wo du es erwähnst, ich könnte es auch über

if(isset($_COOKIE['userid'])) { 

prüfen... sollte es mal überarbeiten... danke

[David]

Empfehlen würde ich, garkein autologin zu programmieren. Dafür haben Browser ihren Passwortmanager.

Aber wenn es schon sein muss, würde ich eine Zufallszeichenkette erstellen lassen, zum Beispiel mit md5($_SERVER['REMOTE_ADDR'].uniqid(null, true))
Diese würde ich dann im Datensatz des Benutzers in der Datenbank speichern. Sozusagen als zweites, temporäres Passwort. Ins Cookie kommt dann diese Zeichenkette und die userid, aber nicht das dauerhafte Passwort. Und die zufällige Zeichenkette würde ich dann regelmäßig verändern. Vielleicht einen Counter in _SESSION und bei jedem xten Aufruf. Außerdem würde das Autologin nicht für kritische Operationen genügen. Also wenn der Benutzer zum Beispiel das Passwort ändern will. Oder überhaupt irgendetwas am Profil drehen will. Wenn also jemand das Cookie klaut, dann nutzt es nicht allzu lange etwas und man kann nicht allzu viel anstellen.

[Papst]

Empfehlen würde ich, garkein autologin zu programmieren. Dafür haben Browser ihren Passwortmanager.

Aber wenn es schon sein muss, würde ich eine Zufallszeichenkette erstellen lassen, zum Beispiel mit md5($_SERVER['REMOTE_ADDR'].uniqid(null, true))
Diese würde ich dann im Datensatz des Benutzers in der Datenbank speichern. Sozusagen als zweites, temporäres Passwort. Ins Cookie kommt dann diese Zeichenkette und die userid, aber nicht das dauerhafte Passwort. Und die zufällige Zeichenkette würde ich dann regelmäßig verändern. Vielleicht einen Counter in _SESSION und bei jedem xten Aufruf. Außerdem würde das Autologin nicht für kritische Operationen genügen. Also wenn der Benutzer zum Beispiel das Passwort ändern will. Oder überhaupt irgendetwas am Profil drehen will. Wenn also jemand das Cookie klaut, dann nutzt es nicht allzu lange etwas und man kann nicht allzu viel anstellen.

An sich habe ich es auch so gemacht.. nur die Userid zusätzlich zu speichern halte ich für Mist bzw unnötig... Ich denke man sollte eher die wenigsten Informationen "herausgeben". Es reicht doch einfach diese Autologin-Session in der DB als "Unique" zu deklarieren und beim eintragen darauf achten, dass sie nur einmal vorkommt. Mit entsprechender Länge (je nach Besucherzahl --> Bei 10k Usern als Beispiel ist es unsinnig einen 10 zeichen-String zu benutzen, da sind die Überschneidungen zu groß) gewinnt das ganze dann wieder ein bisschen an Sicherheit.

[DJ Nuno]

Wenn also jemand das Cookie klaut, dann nutzt es nicht allzu lange etwas und man kann nicht allzu viel anstellen.

Wenn jemand das Cookie klaut, hat der User doch selbst schuld. Meine aufgabe als webmaster ist es zu verhindern, das jemand an user daten kommt weil ich schlampig programmiert habe. Falls also jemand in seinen rechner eindringt, weil er ihn nicht genug gesichert hat, dann ist es die schuld des users und nicht meine.

Aber das mit dem zufälligen code finde ich eine gute idee, dann lasse ich den code bei jedem automatischen neu Login ändern, ich werde wohl einen 20 Stelligen String erzeugen lassen.

Was die userid angeht die bekommt auch jeder zu sehen der sich ein profiel anguckt, das sollte nicht das problem sein da SQL Injection nun nicht mehr möglich ist auf meiner page, eben so wenig wie XSS.

Auf jeden fall bedanke ich mich schon mal sehr für die große hilfe und ideen, wenn jemanden noch etwas einfällt würde ich mich freuen

[David]

Es reicht doch einfach diese Autologin-Session in der DB als "Unique" zu deklarieren und beim eintragen darauf achten, dass sie nur einmal vorkommt.

Ja, das würde auch reichen. Aber dann muss ich eben auch darauf achten, dass der Wert eindeutig ist. Den Aufwand ist mir das nicht wert.

Mit entsprechender Länge (je nach Besucherzahl --> Bei 10k Usern als Beispiel ist es unsinnig einen 10 zeichen-String zu benutzen, da sind die Überschneidungen zu groß) gewinnt das ganze dann wieder ein bisschen an Sicherheit.

Das sehe ich allerdings nicht. Die userid ist doch kein Geheimnis. Meine hier ist übrigens 142, wie man zum beispiel anhand von http://www.php.de/forum/search.php?do=finduser&u=142 erkennen kann. Schlimm! Und meinen Benutzernamen kannst Du auch sehen. Was für eine Sicherheitslücke. uid+Randomstring sind eindeutig und sicher genug. Bei genügend großer Benutzeranzahl wird Dein System sogar weniger sicher, da ich als böser Angreifer weiß, dass jeder Schlüssel nur einmal vorkommt, wohin gegen bei meiner Methode der Schlüsselraum immer gleich groß bleibt. Aber das hat wohl nur theoretischen Charakter.

Wenn jemand das Cookie klaut, hat der User doch selbst schuld. Meine aufgabe als webmaster ist es zu verhindern, das jemand an user daten kommt weil ich schlampig programmiert habe.

Nein, Deine Aufgabe ist es auch, Normalbenutzer vor vermeidbaren Gefahren zu schützen. Ein ganz klein wenig über den Tellerrand zu gucken, gehört einfach dazu.

[DJ Nuno]

Nein, Deine Aufgabe ist es auch, Normalbenutzer vor vermeidbaren Gefahren zu schützen. Ein ganz klein wenig über den Tellerrand zu gucken, gehört einfach dazu.

Alles was ich tun kann ist zu versuchen es so sicher wie möglich zu machen, wenn ein user nicht auf seine Daten aufpasst dann ist er selbst schuld.

[David]

Aber Du kannst den Datenklau ohne viel Aufwand verhindern oder zumindest erheblich erschweren. Sowas passiert wirklich. Öffentliche PCs, unverschlüsseltes wlan, um nur mal zwei Beispiele zu nennen. Und lass es mich ganz offen sagen: DU reißt diese Lücke mit dem autologin überhaupt erst auf*. Also ist es auch DEINE Aufgabe, es wenigstens so sicher wie möglich zu machen. Selbst wenn Du keinerlei Anteil an der Lücke hättest, wäre es Deine Aufgabe, da Du Dich mit dem Thema beschäftigst, und die meisten Benutzer eben nicht. Sorgfaltspflicht gilt auch in der Softwareentwicklung. Dass diese Sensibilität bei der Webentwicklung oft fehlt, macht mir ehrlich gesagt Sorgen. Dass nicht immer fehlerfreie, optimale Software geschrieben wird, ist mir klar und finde ich auch nicht dramatisch, ist bei mir selbst ja nicht anders. Aber die Haltung "passiert nicht", "pah, nicht meine Schuld" oder "das muss doch jeder wissen, da mache ich mir keinen Stress" wenn man es erklärt bekommt, ist schon ärgerlich. Gerade wenn der Aufwand nicht wirklich höher ist. Also, langer Rede kurzer Sinn

Alles was ich tun kann ist zu versuchen es so sicher wie möglich zu machen

tu das.


*) andere cookie-basierte autologins natürlich auch.