[PHP]Czy jest to wystarczające zabezpieczenie? - autoryzacja Opcje

[b_chmura]

Witam

Chciałbym się zapytać was ludzi z doświadczeniem i pomysłem czy takie zabezpieczenie jest wystarczające:

[PHP] pobierz, plaintext 
<?php
//logowanie - jeśli login i hasło są poprawne tworzy się sesja zakodowanego przez md5 loginu 
i hasla
function loguj($login, $pass) 
{
  $spr = mysql_query("SELECT * FROM `USER` WHERE `login` = 'login' AND `haslo` = '$pass'" LIMIT 0,1);
 
  if(mysql_num_rows($spr) == 1)
  {
	$_SESSION['login'] = md5($login.$pass);
	return true;
  }
  else
  {
	return false;
  }
}
//sprawdzanie czy user jest zalogowany - jeśli dane wyciągnięte i zakodowane przez md5 zgadzają sie z sesją wszystko jes
t ok
function spr_usr()
{
  $query = mysql_query("SELECT * FROM `USER` WHERE `login` = '$login' AND `haslo` = '$pass'");
 
  while($user  = mysql_fetch_array($query))
  {
	if(md5($user['login'].$user['haslo']) = $_SESSION['login'])
	{
	  return true;
	}
	else
	{
	  return false;
	}
  }
}
?>
[PHP] pobierz, plaintext 

przejdzie to?

[cornholio666]

Tu chyba masz błąd przy 'login'

[PHP] pobierz, plaintext 
<?php
$spr = mysql_query("SELECT * FROM `USER` WHERE `login` = 'login' AND `haslo` = '$pass'" LIMIT 0,1);
?>
[PHP] pobierz, plaintext 

Do czego to służy:

[PHP] pobierz, plaintext 
<?php
$_SESSION['login'] = md5($login.$pass);
?>
[PHP] pobierz, plaintext 

Chyba dość niewydajne:

[PHP] pobierz, plaintext 
<?php
if(md5($user['login'].$user['haslo']) = $_SESSION['login'])
?>
[PHP] pobierz, plaintext 

Po co az 2 zapytania ?

[b_chmura]

ad1
tak powinno być `login` = '$login'

ad2
Przypisuje sesji "login" wartość

ad3
powinno być:

[PHP] pobierz, plaintext 
<?php
if(md5($user['login'].$user['haslo']) == $_SESSION['login'])
?>
[PHP] pobierz, plaintext 

ad4
bo są to osobne funkcje

Ten post edytował b_chmura 11.09.2007, 15:10:55

[Puciek]

Ale nie wyjasniles po jaka cholere dane do sesji cisniesz w md5.
Dalej, skąd sie bierze $login i $pass ? Pobierasz je z $_POST ? z $_GET ? z maszyny losujacej ? Czy poprostu w pelni radosnie jedziesz na register_globals ?
Stringi lepiej robic apostrofami, czyli

[PHP] pobierz, plaintext 
<?php
mysql_query( 'SELECT * FROM `USER` WHERE `login` = "'.$login.'" AND `haslo` = "'.$pass.'" LIMIT 0,1 ');
?>
[PHP] pobierz, plaintext 

Ten post edytował Puciek 11.09.2007, 15:17:36

[syntax]

pakowanie hash'a do sesji jest troche bez sensu, ja bym to wszytsko zrobil w jednej funkcji.

[cornholio666]

ad4
bo są to osobne funkcje

Po co ci aż dwa zapytania żeby sprawdzic czy uzytkownik istnieje? nie da sie jednym ?

Tutaj w pętli haszujesz login i haslo, czyli jak baza bedzie miala 100000 userów to troche niewydajne bedzie.

[PHP] pobierz, plaintext 
<?php
if(md5($user['login'].$user['haslo']) == $_SESSION['login'])
?>
[PHP] pobierz, plaintext 

Z kodu wynika ze trzymasz hasło w bazie w postaci jawnej a to duzy błąd

[b_chmura]

a czy w ten sposób odpowiednio sie zabezpieczę?

[PHP] pobierz, plaintext 
<?php
function check_user($login, $pass)
{
  if(empty($_SESSION['login']))
  {
	if(empty($login) OR empty($pass))
	{
	  echo 'Nie podałeś Loginu i/lub Nicka';
	  return false;
	}
	else
	{
	  $dane = mysql_query("SELECT * FROM `USER` WHERE `login` = '".$login."' AND `haslo` = '".$pass."' LIMIT 0, 1" );
 
	  if(mysql_num_rows($dane) == 1)
	  {		
		$_SESSION['login'] = 'zalogowany';
 
		echo $login.' poprawnie sie zalogował';
		return true;
	  }
	  else
	  {
		echo 'Login i/lub Hasło są nieprawidłowe!';
		return false;
	  }
	}
  }
  else
  {
	return true;
  }
}
?>
[PHP] pobierz, plaintext 

zmienna $login i $pass pobierane są z formularza.

[cornholio666]

[PHP] pobierz, plaintext 
<?php
if(!isset($_SESSION['login']){
   $_SESSION['login'] = false;
}
 
function check_user($login, $pass)
{
 
 
	if(empty($login) OR empty($pass))
	{
	  echo 'Nie podałeś Loginu i/lub Nicka';
	  return false;
	}
	else
	{
	  $login = mysql_escape_string($login);
	  $pass = md5($pass);
 
	  $dane = mysql_query("SELECT * FROM `USER` WHERE `login` = '".$login."' AND `haslo` = '".$pass."' LIMIT 0, 1" );
 
	  if(mysql_num_rows($dane) == 1)
	  {		
		$_SESSION['login'] = true;
 
		echo $login.' poprawnie sie zalogował';
		return true;
	  }
	  else
	  {
		echo 'Login i/lub Hasło są nieprawidłowe!';
		return false;
	  }
 
  }
 
}
?>
[PHP] pobierz, plaintext 

mniej wiecej...

Ten post edytował cornholio666 11.09.2007, 16:01:34

[b_chmura]

a jakie jest prawdopodobieństwo ze ktoś się "włamie" nie posiadając poprawnych danych ( w tym wypadku)

[cornholio666]

Przeczytaj to http://forum.php.pl/index.php?showtopic=23258

to http://www.gajdaw.pl/varia/xss.html

Przetestuj skrypt

[erix]

Co do danych w sesji: nie musisz trzymać danych zahaszowanych w sesji, ale lepiej, jeśli katalog z danymi przeniesiesz w "niepubliczne" miejsce (bo domyślnie /tmp).

No i obowiązkowo to, o czym cornholio666 wspomniał.

[templar]

Witam

Chciałbym się zapytać was ludzi z doświadczeniem i pomysłem czy takie zabezpieczenie jest wystarczające:

[PHP] pobierz, plaintext 
<?php
//logowanie - jeśli login i hasło są poprawne tworzy się sesja zakodowanego przez md5 loginu 
i hasla
function loguj($login, $pass) 
{
  $spr = mysql_query("SELECT * FROM `USER` WHERE `login` = 'login' AND `haslo` = '$pass'" LIMIT 0,1);
 
  if(mysql_num_rows($spr) == 1)
  {
	$_SESSION['login'] = md5($login.$pass);
	return true;
  }
  else
  {
	return false;
  }
}
//sprawdzanie czy user jest zalogowany - jeśli dane wyciągnięte i zakodowane przez md5 zgadzają sie z sesją wszystko jes
t ok
function spr_usr()
{
  $query = mysql_query("SELECT * FROM `USER` WHERE `login` = '$login' AND `haslo` = '$pass'");
 
  while($user  = mysql_fetch_array($query))
  {
	if(md5($user['login'].$user['haslo']) = $_SESSION['login'])
	{
	  return true;
	}
	else
	{
	  return false;
	}
  }
}
?>
[PHP] pobierz, plaintext 

przejdzie to?

Przed:

[PHP] pobierz, plaintext 
<?php
$spr = mysql_query("SELECT * FROM `USER` WHERE `login` = 'login' AND `haslo` = '$pass'" LIMIT 0,1);
?>
[PHP] pobierz, plaintext 

Zapodałbym:

$login = str_replace("'", '', $login);
$pass = str_replace("'", '', $pass);

Nie ma co się bawić addslashes'ami i innymi badziewiami, lepiej po prostu wyciąć ten apostrof w cholerę - tak jest najbezpieczniej.
I najlepiej robić to wszędzie tam, gdzie zmienna bierze udział w zapytaniu MySQL - chyba, że te apostrofy są potrzebne, wtedy lepiej
zastąpić je funkcją htmlentities() i dopiero zapisywać do bazy.

Pozdrawiam.