Login Script Sicherheit

fr33z3r

Grünschnabel
Hallo zusammen,
habe gerade ein loginscript programmiert und wollte es von euch auf sicherheitslücken prüfen lassen. :) ... wenn also der eine oder andere ein paar worte dazu verlieren könnte wärs nice :]
Wenn jmd das script selbst verwenden will, kann er dies gerne tun.

PHP:
<?php
        session_start();

        if(!isset($_SESSION['IP'])) {
            $_SESSION['IP'] = $_SERVER['REMOTE_ADDR'];
        }

        if($_SESSION['IP'] != $_SERVER['REMOTE_ADDR']) {
            echo "Sie dürfen nicht die Session von einem<br>anderen user Benutzten. Bitte benutzen sie<br>folgenden Link um zur Homepage zu gelangen.<br><a href=\"http://www.meineseite.de\">Zurück zur Homepage</a>";
            die();
        }

	echo $_SESSION[logged_user_id] . " - " . $_SESSION[logged_user_pw] . " - " . session_id() . "<br><br><hr>";
	
	?>
				<form action="index.php" method="post">

					<input type="submit" value="logout">

					<input type="hidden" name="nav" value="haendler_login">
					<input type="hidden" name="show" value="logout">
				</form>
	<?

	switch($show){
		default:
			if(!isset($_SESSION["logged_user_id"]) || !isset($_SESSION["logged_user_pw"])) {

				?>
				<form action="index.php" method="post">
					<center>
					<table>
						<tr>
							<td>User:</td>
							<td><input type="text" name="user"></td>
						</tr>
						<tr>
							<td>Passwort:</td>
							<td><input type="password" name="pw"></td>
						</tr>
						<tr>
							<td></td>
							<td align=center><input type="submit" value="login">&nbsp;<input type="reset" value="reset"></td>
						</tr>
					</table>
					</center>

					<input type="hidden" name="nav" value="haendler_login">
					<input type="hidden" name="show" value="login">
					<input type="hidden" name="PHPSESSID" value="<? echo session_id(); ?>"> 
				</form>

				<?

			} else {
				print "<meta http-equiv=\"refresh\" content=\"0; URL=index.php?nav=haendler_login&show=haendler_area&PHPSESSID=" . session_id() . "\">";
			}
		break;

		case "login":
			//LOGIN PASSWORT CHECK
			$valid_chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
			$valid_chars_size = strlen($valid_chars);
			$password_size = strlen($_POST[pw]);
			$user_password = strtoupper($_POST[pw]);
			$count = 0;
			while($count < $password_size) {

				$count2 = 0;

				while($count2 < $valid_chars_size) {
					
					if($user_password[$count] == $valid_chars[$count2]) {
						$cr8pw .= $valid_chars[$count2];
						$count2 = $valid_chars_size;
					} else {
						$count2++;
					}
				}

				$count++;
			}


			//LOGIN USERNAME CHECK
			$valid_chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789";
			$valid_chars_size = strlen($valid_chars);
			$user_size = strlen($_POST[user]);
			$user = strtoupper($_POST[user]);
			$count = 0;
			while($count < $user_size) {

				$count2 = 0;

				while($count2 < $valid_chars_size) {
					
					if($user[$count] == $valid_chars[$count2]) {
						$cr8user .= $valid_chars[$count2];
						$count2 = $valid_chars_size;
					} else {
						$count2++;
					}
				}

				$count++;
			}


			$result = mysql_query("SELECT kundennummer , password FROM $us_haendler WHERE kundennummer = $cr8user");
			$out = mysql_fetch_array($result);

			if($cr8user == $out[kundennummer] && $cr8pw == $out[password]) {
				$_SESSION["logged_user_id"] = $cr8user;
				$_SESSION["logged_user_pw"] = md5($cr8pw);
				print "<meta http-equiv=\"refresh\" content=\"0; URL=index.php?nav=haendler_login&show=haendler_area&PHPSESSID=" . session_id() . "\">";
			} else {
				print "Falsche Logindaten eingegeben";
				print "<meta http-equiv=\"refresh\" content=\"10; URL=index.php?nav=haendler_login&show=logout\">";
			}
		break;

		case "logout":
			$_SESSION = array();

			if (isset($_COOKIE[session_name()])) {
				setcookie(session_name(), '', time()-42000, '/');
			}

			session_destroy();
			print "<meta http-equiv=\"refresh\" content=\"0; URL=index.php?nav=haendler_login\">";
		break;

		case "haendler_area":
  			$result = mysql_query("SELECT kundennummer , password FROM $us_haendler WHERE kundennummer = $_SESSION[logged_user_id]");
			$out = mysql_fetch_array($result);
			if($_SESSION["logged_user_id"] == $out[kundennummer] && $_SESSION["logged_user_pw"] == md5($out[password])) {
				echo "yo es geht :) ";
			} else {
				echo "Sie haben keine Berechtigung diesen Bereich zu betreten.<br>Scheinbar gibt es Sicherheitslücken mit Ihrer Session oder Ihren Cookies.";
			}
		break;
	}
?>
 
Zuletzt bearbeitet:
Hmm bei deinem Script darf beim Password keine sonderzeichen verwendet werden.Sowie ist bei dir keine klein ,groß schreibung möglich im Password.
Machne geben aber extra sonderzeichen ein um ihr pw sicherer zu machen gegen bruteforce angriffen.Sowie oft auch groß klein schreibung verwendet wird um die sicherheit des pw zu gewähren.

Ich glaube bruteforce angriffe hätte da eine freude bei deinem script da man viel wenigere Zeichen prüfen muß um das pw zu erratten.Bei 7 stellen nur zahlen hat man das pw ich ca 1 std je nach Rechengeschwindigkeit.

Mfg Splasch
 
Zuletzt bearbeitet:
danke für deine schnelle antwort.
Du hast Recht, das PW würde sich per bruteforce unter aktuell angegebener einstellung recht schnell knacken lassen, doch möchte ich keine Unterscheidung zwischen Groß und Kleinbuchstaben. Das hängt mit meiner Kundengruppe zusammen welche dieses Script im Endeffekt benutzt.
Da allerdings mein Kennwort auto. generiert wird ist das mit dem Bruteforce nicht so tragisch. Um gegen sowas vorzugehen werde ich allerdings eine Wartezeit nach Falscheingabe erstellen.

Mir gehts primär um meine Sicherheit im Bereich "sql injection" bzw "session diebstahl" oder Ausnutzung anderer mir bisher nicht bekannter Schwachstellen.

Freue mich auf weitere comments. :)
 
Die sicherste Methode um vor Sql injection vorzubeugen ist es den übergeben Wert erst garnicht ist das Sql State einzubinden.

Nun wirste dich fragen wie soll das gehen.Und es gibt tatsächlich eine Möglichkeit wie man das Realiesieren kann.Allerdings muß ich dazu auch sagen das die Methode etwas Server lastig ist. Aber sie ist zu 99,9% efektive gegen Sql injection.
Und dabei ist es dann egal welche zeichen mit übergeben werden.

Mfg Splasch
 
Wenn du nicht direkt auf die Eingabe achten musst, sprich was eingegeben wird, kannstt du die funktion htmlspecialchars() verwenden, somit wird gegen SQL Injects gearbeitet. Union etc sind nichtmehr möglich dann, XSS ebenfalls nicht.

Gegen den Sessionklau würde ich es genauso machen, zur ID eien IP zuweisen und prüfen.
 
Die htmlspecialchars()-Funktion schützt eben nicht vor SQL-Injektionen. Denn diese maskiert nur die HTML-Metazeichen, nicht aber die MySQL-Metazeichen. Gerade dafür gibt es die mysql_real_escape_string()-Funktion, die auch die Zeichenkodierung der MySQL-Verbindung berücksichtigt.

Doch auch ich empfehle dir, die Anzahl der möglichen Zeichen nicht zu sehr einzuschränken. Da wäre es sinnvoller eine Mindestanzahl der Zeichen des Kennworts festzulegen, um keine zu kurzen Kennwörter zuzulassen.

Die komplizierten Konstrukte, die prüfen ob nur erlaubte Zeichen verwendet wurde, sind übrigens überflüssig. Ein binärer Vergleich (siehe BINARY-Operator) bewirkt bei der Datenbankabfrage dasselbe, wenn die Werte in der Datenbank ebenfalls nur aus den erlaubten Zeichen bestehen.
 
danke gumbo, hat mir sehr geholfen. :)
Hier für euch das ergebniss nochmal überarbeitet, danke für eure hilfe.
Hoffe dieses Script ist noch ein funken sicherer als die standart scripts, sonst war mein ganzer auffwand umsonst.
Wie gehabt dürft ihr es auch gerne für eure Homepage verwenden.
Gruß fr33z3r

PHP:
<?php
        session_start();

	function check_user_input($string) { 
		$valid_chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789";
		$valid_chars_size = strlen($valid_chars);
		$size = strlen($string);
		$user_data = mysql_real_escape_string($string);
		$count = 0;
		while($count < $size) {
			$count2 = 0;
			while($count2 < $valid_chars_size) {
					
				if($user_data[$count] == $valid_chars[$count2]) {
					$result .= $valid_chars[$count2];
					$count2 = $valid_chars_size;
				} else {
					$count2++;
				}
			}
			$count++;
		}
		return $result; 
	}

        if(!isset($_SESSION['IP'])) {
            $_SESSION['IP'] = $_SERVER['REMOTE_ADDR'];
        }

        if($_SESSION['IP'] != $_SERVER['REMOTE_ADDR']) {
            echo "Sie dürfen nicht die Session von einem<br>anderen user Benutzten. Bitte benutzen sie<br>folgenden Link um zur Homepage zu gelangen.<br><a href=\"http://www.meineseite.de\">Zurück zur Homepage</a>";
            die();
        }

	echo $_SESSION[logged_user_id] . " - " . $_SESSION[logged_user_pw] . " - " . session_id() . "<br><br>";
	
	?>
				<form action="index.php" method="post">

					<input type="submit" value="logout">

					<input type="hidden" name="nav" value="haendler_login">
					<input type="hidden" name="show" value="logout">
				</form><hr>
	<?

	switch($show){
		default:
			if(!isset($_SESSION["logged_user_id"]) || !isset($_SESSION["logged_user_pw"])) {

				?>
				<form action="index.php" method="post">
					<center>
					<table>
						<tr>
							<td>User:</td>
							<td><input type="text" name="user"></td>
						</tr>
						<tr>
							<td>Passwort:</td>
							<td><input type="password" name="pw"></td>
						</tr>
						<tr>
							<td></td>
							<td align=center><input type="submit" value="login">&nbsp;<input type="reset" value="reset"></td>
						</tr>
					</table>
					</center>

					<input type="hidden" name="nav" value="haendler_login">
					<input type="hidden" name="show" value="login">
					<input type="hidden" name="PHPSESSID" value="<? echo session_id(); ?>"> 
				</form>

				<?

			} else {
				print "<meta http-equiv=\"refresh\" content=\"0; URL=index.php?nav=haendler_login&show=haendler_area&PHPSESSID=" . session_id() . "\">";
			}
		break;

		case "login":
			$user = check_user_input($_POST[user]);
			$pw = check_user_input($_POST[pw]);
			$result = mysql_query("SELECT kundennummer , password FROM $us_haendler WHERE kundennummer = $user");
			$out = mysql_fetch_array($result);

			if($user == $out[kundennummer] && $pw == $out[password]) {
				$_SESSION["logged_user_id"] = $user;
				$_SESSION["logged_user_pw"] = md5($pw);
				print "<meta http-equiv=\"refresh\" content=\"0; URL=index.php?nav=haendler_login&show=haendler_area&PHPSESSID=" . session_id() . "\">";
			} else {
				print "Falsche Logindaten eingegeben";
				print "<meta http-equiv=\"refresh\" content=\"10; URL=index.php?nav=haendler_login&show=logout\">";
			}
		break;

		case "logout":
			$_SESSION = array();

			if (isset($_COOKIE[session_name()])) {
				setcookie(session_name(), '', time()-42000, '/');
			}

			session_destroy();
			print "<meta http-equiv=\"refresh\" content=\"0; URL=index.php?nav=haendler_login\">";
		break;

		case "haendler_area":
  			$result = mysql_query("SELECT kundennummer , password FROM $us_haendler WHERE kundennummer = $_SESSION[logged_user_id]");
			$out = mysql_fetch_array($result);
			if(mysql_real_escape_string($_SESSION["logged_user_id"]) == $out[kundennummer] && mysql_real_escape_string($_SESSION["logged_user_pw"]) == md5($out[password])) {
				echo "yo es geht :) ";
			} else {
				echo "Sie haben keine Berechtigung diesen Bereich zu betreten.<br>Scheinbar gibt es Sicherheitslücken mit Ihrer Session oder Ihren Cookies.";
			}
		break;
	}
?>
 
Ich sagte doch, dass das umständliche Konstrukt, das du nun in der check_user_input()-Funktion ausgelagert hast, nicht nötig ist, wenn du einen binären Vergleich nimmst.
Andernfalls wäre auch ein regulärer Ausdruck oder ein Konstrukt mit der strpbrk()-Funktion einfacher:
PHP:
!preg_match('/[^A-Za-z0-9]/', $str)
!strpbrk($str, 'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789')
Darüber hinaus sollte das Skript Fehleingaben nicht einfach korrigieren sondern sie dem Benutzer melden. Auch ist es unnötig das Passwort in der Sitzung zu speichern.
 
Zusätzlich könntest du noch eine Anti Bruteforce Funktion einbauen.
Hierzu findest du ein Tutorial hier.
Es geht zwar eigentlich um ein Loginsystem und OOP, aber diesen Teil könnte man ja auch noch einbauen. Sieht meiner Meinung nach ganz gut aus.

Gruß
Marc
 

Neue Beiträge

Zurück