Er dette simple login-scriptet sikkert nok?

[Twinflower]

Siden PHP var noe jeg "lærte" meg på #php-kanalen på begynnelsen av 2000-tallet, og for det meste har dreid seg om å hente og lagre enkle saker i databaser, så er jeg ikke helt oppdatert på ting som login og slike "avanserte" greier.

 

 

Men nå har jeg etter masse dilling og læring og "upload-overwrite-F5"-ing fått til noe som faktisk fungerer.

 

Spørsmålet er om det er bra nok, noe dere sikkert kan gi meg noen inputs på.

 

 

Her er scriptet:

<?PHP
if (empty($_SESSION["Login"])) { // Hvis personen ikke allerede er innlogget, kjør login-script
function login() { // funksjon HTML-en for å slippe å fylle kildefilen med samme kode flere ganger
	echo "
	<form  method=\"post\" action=\"\">
	<div id=\"login\">
	<input type=\"hidden\" value=\"login\" name=\"action\" /><br/>
	<p>NAK-nr:</p>
	<input name=\"naknr\" type=\"text\" id=\"naknr\" /><br/>
	<p>Passord:</p>
	<input name=\"passord\" type=\"password\" id=\"passord\" /><br/>
	<br/>
	<input type=\"submit\" name=\"Submit\" value=\"Login\"/>
	</div>
	</form>";
}
if (!empty($_POST["action"]) && $_POST["action"] == "login") { //Hvis skjema er fyllt ut og submittet, kjør dette
	$action = $_POST["action"]; 
	$naknr = $_POST["naknr"];
	$naknr = stripslashes($naknr); // Beskyttelse mot mysql injection
	$naknr = mysql_real_escape_string($naknr); // Beskyttelse mot mysql injection
	$passord = $_POST["passord"]; 
	$passord = mysql_real_escape_string($passord); // Beskyttelse mot mysql injection
	$passord = stripslashes($passord); // Beskyttelse mot mysql injection
	$login_query = mysql_query("SELECT * FROM foniks_members WHERE naknr = '$naknr' AND password = '$passord'") or die("<div class=\"error\">MySQL error: " . mysql_error() . "</div>"); // Søke i databasen etter treff på brukernavn og passord
	$login_info = mysql_fetch_array($login_query); // Behandle databasespørringresultat 
	if (mysql_affected_rows() == 0) { //Hvis ingen treff, gi feilmelding
		echo "<p class\"error\">Nak-nummer og passord stemmer ikke overens</p>";
		login(); //echoes the login-form
	}
	else { // Hvis treff, sett login-session lik NAK-nr
		$_SESSION["Login"] = $login_info[naknr];
		Header("location: /site"); // Redirect til start-side
	}
}
else login(); // Hvis skjema ikke er fyllt ut, vis kun skjema
}

else { // Hvis noen allerede er logget inn når scriptet kjøres, slett session, return til startside (aka logg ut)
$_SESSION["Login"] = "";
unset($_SESSION["Login"]);
Header("location: /site");
}
?>

 

 

Her er også scriptet, med php-color coding: http://pastebin.com/m1620aaea

 

Jeg har ikke laget noen beskyttelse mot bruteforce enda, og vet ikke om jeg helt behøver det.

Dette er en fallskjermklubb-side, med 40-50 medlemmer hvor ganske få kommer til å logge seg inn spesielt ofte.

 

NAK-nr er forresten brukernavn.

Endret 16. desember 2008 av Twinflower

[oxodesign]

Ja den der er OK, men for å få litt ekstra sikkerhet kan du foreks lage en key som blir kryptert som du sjekker imot om en session er virkelig den sessionen den sier den er!

 

eks. på det "md5($dinHemmeligKey.$naknr)" lagre den i en session variable og sjekke om den stemmer med md5($dinHemmeligKey.$_SESSION['Login']).

 

jeg tok litt tid å re-laget skriptet ditt:

 

<?php
function escapeInput($str){
return get_magic_quotes_gpc() ? stripslashes($str) : $str; 
}

function login(){
?> 
<form method="post" action="">
	<div id="login">
		<input type="hidden" value="login" name="action" /><br/>
		<p>NAK-nr:</p>
		<input name="naknr" type="text" id="naknr" /><br/>
		<p>Passord:</p>
		<input name="passord" type="password" id="passord" /><br/>
		<br/>
		<input type="submit" name="Submit" value="Login"/>
	</div>
</form>
<?php 
}

function logout(){
unset($_SESSION["Login"]);
session_destroy();

Header("location: /site");
}

if(isset($_SESSION['Login'])){
logout();
}else{
if(isset($_POST['action']) && $_POST["action"] == "login"){
	$p_naknr 	= escapeInput($_POST['naknr']);
	$p_passord 	= escapeInput($_POST['passord']);

	$result		= mysql_query(sprintf("SELECT * FROM foniks_members WHERE naknr='%s' AND password='%s' LIMIT 1",
								 mysql_real_escape_string($p_naknr),
								 mysql_real_escape_string($p_passord)));
	if(mysql_num_rows($result) == 1){
		$_SESSION['Login'] = $p_naknr;
		Header("location: /site");
	}else{
		echo "<p class\"error\">Nak-nummer og passord stemmer ikke overens</p>";
		login();
	}
}else login();
}
?>

[Twinflower]

Tusen takk, men jeg vet ikke om jeg skjønte helt greia med krypteringen og sjekken der.

 

Hvorfor brukte du sprintf() til spørringen?

Endret 16. desember 2008 av Twinflower

[Wackamole]

http://no.php.net/manual/en/function.sprintf.php