Witam.
Męczę się nad bezpiecznym systemem uwierzytelniania i proszę ocenić, czy takie coś będzie bezpieczne:

[PHP] pobierz, plaintext 
<?php
session_start();
 
if(isset($_POST['login']) and isset($_POST['haslo']) and $_GET['action']=='zaloguj')
	{
	$query = mysql_query("select * from `users` where `login`='".mysql_real_escape_string($_POST['login'])."' and `haslo`='".mysql_real_escape_string(md5($_POST['haslo'])).'"');
	if(mysql_num_rows($query)>0)
	   {
	   $_SESSION['logged'] = 1;
	   $_SESSION['username'] = $_POST['login'];
	   }
	else
		{
		$_SESSION['logged']=0;
		{
	}
 
   if($_SESSION['logged']==1)
	  {
	   /* JAKIŚ TAM PANEL ADMINISTRACYJNY */
	  }
 
?>
[PHP] pobierz, plaintext 

Głównie chodzi mi o to, czy ktoś może zmienić wartość tablicy sesji $_SESSION['logged'] na 1 i tym samym zdobyć uprawnienia admina.

Ja bym zmienil jak juz cos jedna rzecz zamiast dawac mysql_num_rows() wez sobie za pomoca mysql_fetch_assoc() sprawdzaj jeszcze raz dane to raz a dwa dodal bym else z header'em na google gdy ktos poda zle dane wtedy nie bedzie nic widzial etc ogolnie jak trzymasz identifikator w url to generuj za kazdym razem nowe id dla sesji i potem na kazdej podstawie sprawdzaj czy jest dany login z sesji w bazie

Umieściłem tylko przykład, dlatego nie dodawałem else z informacją typu "błędny login lub hasło" a identyfikator sesji przecież nie jest przekazywany w url-u, no chyba, że coś przeoczyłem

jeszcze zabezpieczenie przed XSS by można było ^^

Nie jest, ale czy dyrektywa session.use.cookies.only jest włączona? Jeśli nie identyfikator pójdzie przez adres w przypadku gdy cookie nie zostanie przyjęte. Także regeneracja nie jest aż tak zbędna, jakby się wydawało, chociaż dzisiaj praktycznie większość serwerów wymusza sesje przez ciacha ^^

@up
A gdzie tu masz wypisywanie jakichkolwiek danych wprowadzonych przez użytkownika?

Nie umieszczałem formularza, bo chodzi mi głównie o kod php Ale jak to ma się na coś zdać to dodałbym tam np.

[HTML] pobierz, plaintext 
<form action="login.php?action=login" method=post>
login <input type=text name=login><br>
hasło <input type=password name=haslo<br>
<input type=submit value=zaloguj>
</form>
[HTML] pobierz, plaintext 

Nie nie, to było do @pyro. Powinnam była wyraźniej zaadresować

No tak... to co pokazał autor to jest pewnie cały skrypt, prawda Shili?

Nieprawda, ale przekazywane jest tylko login i hasło (bo i po co więcej do logowania), co można podejrzeć na dodanym przez autora kodzie. Zresztą nie do końca poprawnym, ale to inna bajka. Zresztą...

Bez zobaczenia reszty kodu można gdybać, bo może być tam brak zabezpieczenia do każdego możliwego błędu, autor może nawet wypisywać wszystkie swoje hasła. Ale po co gdybać, jeśli wyraźnie dostaliśmy taki fragment kodu do analizy, jaki MGreg chce by zanalizowano?

Co najwyżej można poprosić o resztę i wtedy się odnosić

Co do XSS stosuję strip_tags(); i chyba to mi wystarczy Jutro wstawię poprawioną wersję i będziemy dyskutować dalej

To jeszcze powiedz autorowi jak powinien zabezpieczyć serwer bo przecież gdzieś to musi uruchomić Autor pisze o systemie autoryzacji a Ty opowiadasz o XSS jakby to miało cokolwiek wspólnego.

Wracając do tematu to zależy co rozumiesz pod słowem "bezpieczny"... Ogólnie przy systemach autoryzacji powinieneś wziąć pod uwagę jeszcze takie rzeczy jak Brute-force i zastosować np.

1. Po 3 nieudanych próbach - kolejne próby dostępne co minutę.
2. Max. 10 nieudanych prób logowania, potem blokada konta, aktywacja poprzez mail do właściciela.

Poza tym, już niezwiązane z brute-force:

1. Zastosować SSL.
2. Uważać na tzw. replay attack poprzez nieużywanie danych, które umożliwiają trwały dostęp do zasobów. Szczególnie tzw. trwały login. Jeśli już używasz takich zabawek to nie trzymać w ciastkach hasła i loginu, nawet zhashowanych, tylko zastosować dodatkowy identyfikator zamiast loginu + token zamiast hasła.
3. Przy logowaniu zastosować hasła maskowane, klawiaturę ekranową...

No i oczywiście SQL Injection.

Wszystko zależy od tego jak bardzo "bezpieczny" chcesz mieć system autoryzacji.

Jeśli chodzi bezpośrednio o kod, który podałeś to:

1. Nie używaj Select *...
2. Dodaj LIMIT do zapytania
3. Nie sprawdzaj mysql_num_rows($query)>0 tylko mysql_num_rows($query)===1 -> zalogowany
4. Dodaj ograniczenie długości dla wprowadzanego hasła i loginu.
5. Nie trzymaj w sesji nazwy użytkownika tylko np. wspomniany już losowy identyfikator...

..i to tyle z rzeczy, które teraz przyszły mi do głowy.

Zobaczcie czy teraz jest lepiej. Dodałem sprawdzanie długości loginu i hasła, wstawiłem session_regenerate_id();, dodałem limit do zapytania, wywaliłem zapisywanie loginu do sesji i dodałem wylogowywanie.

[PHP] pobierz, plaintext 
<?php
session_start();
ob_start();
session_regenerate_id();
include('config.php');
pol_mysql();
 
if($_GET['action']=='logout')
	{
	if($_SESSION['logged']==1)
		{
		$_SESSION['logged']=0;
		echo "<b>zostałeś wylogowany<b>, jeśli nie zostaniesz przeniesiony na stronę logowania kliknij <a href=login.php>tutaj</a>";
		header("refresh: 5,login.php");
		}
	else
		{
		echo "nie byłeś zalogowany!";
		}
	}
 
if(!isset($_SESSION['logged'])){$_SESSION['logged']=0;}
 
if(isset($_POST['login']) and isset($_POST['haslo']))
   {	   
   if(strlen($_POST['login'])<25 and strlen($_POST['haslo'])<25)
	 {		   
	 $query = mysql_query("select `username`,`user_password` from `phpbb_users` where `username`='".mysql_real_escape_string($_POST['login'])."' and `user_password`='".mysql_real_escape_string(md5($_POST['haslo']))."' limit 1");
	   
	 if(mysql_num_rows($query)===1)
	   {			   
	   $_SESSION['logged'] = 1;	 
	   }
	 else
	  {
	  echo "Błędny login lub hasło";
	  $_SESSION['logged']= 0;
	  }
	 }		   
	 else
	  {
	  echo "Zbyt długi login lub hasło";
	  }	   
   }
	   
   if($_SESSION['logged']==1)
	 {	   
	 echo "Jakiś tam panel admina <a href=login.php?action=logout>Wyloguj</a>";		 
	 }	   
		   
   
 
if($_SESSION['logged']==0 and $_GET['action']!=='logout')
	{
	?>
	<form action="login.php" method=post>
	login <input type=text name=login><br>
	hasło <input type=password name=haslo><br>
	<input type=submit value=zaloguj>
	</form>
	<?
	}
		   
?>
[PHP] pobierz, plaintext 

Czy teraz jest lepiej?

Dlaczego trzymanie nazwy użytkownika w sesji może być niebezpieczne?

Dlatego, że w niektórych przypadkach można ujawnić dane sesji użytkowników. Jeśli atakujący to zrobi i zobaczy jak sesja wygląda, co zawiera, którego użytkownika dotyczy... to będzie miał spore pole do popisu. Szczególnie trzeba uważać na współdzielonych hostingach.

Wracając do tematu... czy może ktoś obiektywnie ocenić, czy ten mój poprawiony system uwierzytelniania jest lepszy niż poprzedni i czy trzeba w nim coś zmienić?

Może być.


session_regenerate_id(true) zamist session_regenerate_id().

Autor pyta o ten kawałek kodu co podał, a tam NIE MA zabezpieczenia przed XSS, dlatego o tym powiedziałem.


Niby dlaczego nie?

Co to ma dać? Dłuższe zapytanie?

W przypadku tego kawałka kodu co to za różnica? Lecz powinno się też sprawdzać czy użytkownik już istnieje.

A co? Uważasz, że to niebezpieczne?




To może nie używajmy wogóle baz danych, plików txtowych ani wogóle niczego, bo w niektórych przypadkach można je ujawnić?

w

Nie Select * tylko wybrane pola czyli Select pole1, pole2. Chodzi o to aby w przypadku ataku luki itp. można było wyciągnąć dane z tych pól a nie np. z pola password itp. Limit to samo w razie ataku pobierze jednego usera a nie całą tablice userów. A co do sesji to lepiej dmuchać na zimne

Czy chociaż sam rozumiesz co tu napisałeś ^^?

Co to za różnica, tak czy inaczej mozna pobrac całą tablicę userów.


aha... czyli potwierdzają się moje słowa panie "włamywacz"?

Dalej nie ma tam żadnej wzmianki o tym, że autor chce wyświetlać tekst pochodzący z formularza. Już nawet napisałam, że może tam spokojnie umieścić swoje wszystkie hasła, ale póki tego nie zrobi nie ma po co męczyć go stwierdzeniami: pamiętaj, nie wypisuj swoich haseł na stronie

Jedna uwaga - jeśli action jest logout, to mimo wszystko wykona się dalszy kawałek kodu. Co prawda pewnie nic się nie stanie, ale po co maszyna ma mielić coś, czego użytkownik nie ma na myśli.
Spokojnie użyj tam else.

http://www.acros.si/papers/session_fixation.pdf

Prosto z manuala, naprawdę warto tam zaglądać

W takim razie, powiedz mi proszę, jak chciałbyś przeprowadzić atak XSS na ten kawałek kodu, który tam jest podany.



Ehh... nie mam zamiaru Cię przekonywać, chyba nie wiesz za bardzo o czym piszesz, a ja nie mam ani chęci ani czasu na takie zabawy.

Btw. pisze się "w ogóle"