Ist diese Funktion sicher?

Hallo,

hab ein kleines Portal mit Login.
Da mehrere Seiten nur für eingeloggte User zugänglich sind, habe ich mich entschlossen eine Funktion zu schreiben, da sonst immer ein relativ langer Login Code den anderen Code ziemlich unübersichtlich macht.

Also hier erstmal der Code für die Funktion (in functionen.php):

function login($einloggen)
{

//Variablen definieren
$username=$_SESSION['username'];
$userid = $_SESSION['userid'];
$passwortmd5 = $_SESSION['passwort'];


//Benutzerdaten überprüfen
$abfragen  =  mysql_query( "select username,passwort,aktiviert from members WHERE id = '$userid'");
while  ($row  =  mysql_fetch_row($abfragen)) 
{

if($username == $row[0] && $passwortmd5 == $row[1]  && $row[2]=="1")
{
$einloggen=true;
} //if
} //while
return $einloggen;
} //function

Die wird halt dann über include in eine "geschützte" Seite includet:
Hier der Code von der anderen Seite (test.php):

include("functionen.php");

$einloggen = login(false);
if($einloggen == "1")
{
echo"Du bist eingeloggt";
}
else
{
echo"
Du bist nicht eingeloggt!
";
}

Funktioniert auch wunderbar....jetzt nur die Frage ist das sicher? Ich weiss nichts ist mehr 100% sicher....aber ist das sicher genug, dass es Hack-Anfängern nicht möglich ist meine Seite zu hacken und in die geschützen Bereiche zu kommen?

Danke
goosele

Was mir jetzt ganz am Anfang auffällt währe der Name der include Datei.
Ich könnte jetzt einfach: www.deine-Seite.de/login/functionen.php
in den Browser eingeben und müsste nurnoch den Quellcode einsehen. Nimm am besten:"#functionen.php" als Name. Somit versucht der Browser auf die sprungmarke "funktionen.php" zuzugreifen, was nicht geht. Somit kann man nicht auf die Datei zugreifen.

Sorry Code funktioniert doch noch nicht
Kann mir einer sagen, was ich falsch gemacht habe?

EDIT: Funktioniert jetzt Hab den Fehler gefunden! Also der oben angegebene Code ist jetzt richtig! Also wenn ihr noch irgendwelche Sicherheitsbedenken habt, teilt sie mir bitte mit. Danke!

Ach ja wers mal ausprobieren will:
www.klick-reich.de/index2.php?p=test //Dort steht eingeloggt/nicht eingeloggt
www.klick-reich.de/functionen.php //Dort ist die Funktion
Zum einloggen:
User: test
PW: test

Mann muss dann aber manuell wieder auf www.klick-reich.de/test.php



Danke schon mal für den Tipp, werde ich ändern!

Nimm für deine Variablen vielleicht nicht nur Strings. Wieso ich das meine verstehst du wenn du dir anschaust wieviel Speicher für Strings, Integer... reserviert wird. Verweis auf:
$var1 = "true";
-> sondern $var1 = true;

$var2 = "1.23";
-> sondern $var2 = 1.23;

Ok hab ich gerade geändert (auch schon oben im Code)!
Bei true gibt er ja 1 aus....
also musste ich den code von if($einloggen=="true") auf if($einloggen =="1")
ändern....

Wenn nichts zu meckern ist....bitte auch schreiben, damit ich weiss was sache ist.

Danke

Ok ich geh jetzt mal davon aus, dass das sicher ist.
Kann mir das einer,der Ahnung hat bestätigen?
Einach kurz schreiben:
Ist ok!
Dann bin ich beruhigt..

Wie wärs die Include Datei mit einem absoluten Pfad zu includen und dann unterhalb des htdocs, html, www, etc. zu sichern? Dann kann man von außen nicht auf die Datei zugreifen bzw. die Ausgabe des Hauptverzeichnis einen Ordner weiter zu vertiefen.

[gala]

o_______O

man kann doch nicht über den browser ein phpquelltext ansehen!

@script
ob dein script sicher ist weiss ich nicht. wenn du deine geheimen daten nur auf test.php präsentieren willst, also dort wo steht "du bist eingeloggt", dann ist die seite sicher. wenn du aber dort zb. einen link auf ein geheimes portal setzten willst dann ist die seite nicht sicher.

Wann und wie wird $_SESSION['userid'] mit Daten befüllt?

Wieso so kompliziert?
Ich denke, dass (siehe Bruchpilot) die Daten username, passwortmd5 und userid mit einem HTML-Formular beim Login übergeben werden. Was läge jetzt näher, als einmalig beim Login die Rechte zu setzen, da die Session-Daten auf dem Server gespeichert werden und somit als sicher angesehen werden können->setzen einer neuen Variable $_SESSION['loggedin'].
Das spart dir die umfangreiche Abfrage.
Oder ist gemeint, das einmalig mit "test.php" eingeloggt wird und die Variable $einloggen auf den betroffenen Seiten abgefragt wird?

Die Lösung, die ich auf meiner Homepage verwende:

<?php
	function Rights($reqgroup){
		include "group.php"; // enthält die Benutzerrechte in Form von Arrays
		if (in_array($_SESSION['group'], $$reqgroup));
		else
		{
			die("Du hast nicht die erforderlichen Rechte!");
		}
	}
?>

Diese Funktion wird immer aufgerufen, wenn eine geschützte Datei geladen wird. Dazu reicht der Aufruf:

<?php include('script/rights.php'); Rights('admin') ?>

Anmerkung: $_SESSION['group'] bzw. $reqgroup bezeichnen Gruppen, da die Benutzer unterschiedliche Rechte haben. In deinem Fall würde also folgende Funktion reichen:

function Rights();
{
	if(!$_SESSION['loggedin'])
	{
		die('Du bist nicht eingeloggt');
	}
}

Ansonsten ist meiner Meinung nach dein Code sicher. Die einzige Sicherheitslücke besteht in den Session-Daten, wenn die Session-ID, auf welchem Weg auch immer, an einen anderen Nutzer gelangt. Dies lässt sich aber nicht sinnvoll vermeiden.

Gruß,
Helios

nochmal am rande:
nach möglichkeit immer === statt == verwenden

=== bedingt zusätzlich, dass die variable den gleichen typen hat!
der string "true" wäre mit einer booleschen variable true verglichen bei == also true.
mit === ist es false, da der variablentyp sich unterscheidet!