Witam
Prosiłbym o ocenę mojego rozwiązania pod kątem bezpieczeństwa. Szybciej i przyjemniej było mi napisać coś swojego niż godzinami odnajdywać się w czyimś kodzie (brak wcześnijszego kontaktu z PHP). Mam nadzieję, że ma to prawo działać - czyli bronić dostępu do części serwisu.
Dodam, że to moje pierwsze pisanie w PHP więc proszę o zrozumienie jeśli stosowałem nie najkrótsze rozwiązania. Mam nadzieję, że wszystko jest jasne.
Czekam na opinie. Pozdrawiam.

[PHP] pobierz, plaintext 
plik restricted.php
<?
session_start();
 
switch(@$ad){  //$ad - zmienną przekazywane jest żądane miejsce docelowe, w  części ogólnodostępnej serwisu jest to nazwa pliku bez dopełnienia
	case'':				$nag="Brak strony!";		 $ad="brak";		  break;  //$nag to nazwa strony - wykorzystuję ją w treści
	case'druki':		   $nag="Przydatne druki";	  $ad="secur_druki";   break;  //secur_druki.html to docelowa strona do wczytania
	case'adresy':		  $nag="Przydatne aderesy";	$ad="secur_adresy";  break;
	default:			   $nag="Brak strony!";		 $ad="brak";
}
if(!isset($otwarcia))
	$otwarcia = 0;  //jest to liczba otwarć stron z zabezpieczonej części serwisu, za pierwszym razem 
trzeba się zalogować
include("secur.php"); //tutaj jest rozwiązana weryfikacja - w przypadku braku autoryzacji reszta skryptu nie jest wykonywana, tak samo w pr
zypadku logowania i wylogowania, patrz niżej
 
include("menu.html"); //ostateczne budowanie html'a do wyświetlenia
if(!file_Exists("$ad.html"))
  $ad="brak";
include("$ad.html");
include("dol.html");
?>
[PHP] pobierz, plaintext 

[PHP] pobierz, plaintext 
plik secur.php //zawiera kolejno części odpowiedzialne za logowanie, wylogowanie, stronę powitaln
ą po logowaniu, błąd logowania
<? 
function sprawdz($login, $haslo) 
{
	include("passwords.txt");  //plik z tablicą asocjacyjną $loginy: klucze to użytkownicy (login), wartości to hasła
	if(@$loginy["$login"]==$haslo)
	  $weryf="OK";
	else
	  $weryf="NOK";
	return $weryf;  //weryf - weryfikacja
}
 
if(empty($haslo)||empty($login))  //LOGOWANIE
{
	$nag = "Weryfikacja użytkownika"; //nagłówek
	include("menu.html");  //budujemy stronę, bo już nie "wrócimy" do poprzedniego pliku restricted.php, na końcu jest exit
	echo "
	  <tr><td> <!-- TREŚĆ -->
		Informacje na tej stronie przeznaczone są tylko dla Członków.<br>
		<b>Zaloguj się:</b><p>
		<table><tr><td align=\"right\">
			<form method=\"post\" action=\"restricted.php\">	  //czyli "wracamy" do poprzedniego pliku
			<input type=\"hidden\" name=\"ad\" value=\"$ad\">	 //przekazujemy adres do którego chciał trafić user
			Login: <input type=\"text\" name=\"login\"><br>
			Hasło: <input type=\"password\" name=\"haslo\"><br></td>
			<td>&nbsp;<input type=\"submit\" name=\"loguj\" value=\"Loguj!\">
//po kliknięciu submit wczytywany jest raz jeszcze poprzedni plik, jednak tym raze
m bogatszy o zmienne $login i $haslo
		</td></tr></table>
			  </td></tr> <!-- Kon TREŚĆ -->";
	include("dol.html");
	exit;
}
 
else if(@$action=="logoff")  //WYLOGOWANIE, tylko jeśli pojawi się zmienna $action uwalniana przyciskiem "wyloguj"
{
	session_destroy();
 
	$nag = "Wylogowano $login";
	include("menu.html");
 
	echo "
		<tr><td> <!-- TREŚĆ -->
		&nbsp;<p><center>Zostałeś pomyślnie wylogowany.</center>
			  </td></tr> <!-- Kon TREŚĆ -->";
	include("dol.html");
	exit;
}
 
else
$weryf = sprawdz($login, $haslo);  //jeśli hasło lub login nie były puste odpalana jest funkcja
 
if($weryf=="OK")  //jeśli weryfikacja pomyślna
{
	session_register ("login");
	session_register ("haslo");
	session_register ("weryf");
 
	if($otwarcia==0)  //jeśli dopiero co się zalogowaliśmy i mamy true pojawi się poniższe przywitanie
	{
		$st_nag = $nag;  //aby zachować nagłówek (nazwę) strony do której chciał się dostać user, zmienna z adresem jest zachowana
		$nag = "Zalogowano pomyślnie!";  //nazwa aktualnej strony powitalnej
		include("menu.html");
		echo "
			<tr><td> <!-- TREŚĆ -->
			Witamy użytkownika <b>$login!</b><p>
			Możesz teraz korzystać z części serwisu przeznaczonej wyłącznie dla Członków.<p>
			Kliknij tu, aby dostać się do wybranego wcześniej zasobu - <a href=\"restricted.php?ad=$ad\">$st_nag</a>.
			</td></tr> <!-- Kon TREŚĆ -->";
		include("dol.html");
		$otwarcia++;  //zmieniamy  na 1
		session_register ("otwarcia");
		exit;
	}
	session_register ("otwarcia");  //jeśli wszystko się powiodło po prostu zmienne są przekazywane i user trafia tam 
gdzie chciał
}
 
else //jeśli weryfikacja nie powiodła się
{
	$nag = "Weryfikacja użytkownika";
	include("menu.html");
	echo "
		<tr><td> <!-- TREŚĆ -->
		<p>Błędny login lub hasło!<br>Spróbuj raz jeszcze.<p>
		<table><tr><td align=\"right\">
			<form method=\"post\" action=\"restricted.php\">
			<input type=\"hidden\" name=\"ad\" value=\"$ad\">
			Login: <input type=\"text\" name=\"login\"><br>
			Hasło: <input type=\"password\" name=\"haslo\"><br></td>
			<td>&nbsp;<input type=\"submit\" name=\"loguj\" value=\"Loguj!\">
		</td></tr></table>
			  </td></tr> <!-- Kon TREŚĆ -->";
	include("dol.html");
	exit;
}
?>
[PHP] pobierz, plaintext 

KIlka dość poważnych uchybień:

1. restricted.php, linia 11
masz tam teraz if(!isset($otwarcia)), a nie wiadomo skąd wzięła się zmienna $otwarcia. Zapewne w ustawieniach PHP (plik php.ini) masz zmienną register_globals ustawioną na ON. Jest to bardzo zła praktyka i nie zalecana ze względów bezpieczeństwa. Dlaczego? Na tym forum wiele tematów pod hasłem "register_lobals" znajdziesz.

2. secur.php linia 6
include("passwords.txt"); - tu baardzo duża luka... co się stanie jeśli w przeglądarce wpiszesz: http://<domena>/passwords.txt? Założę się, że w tym przypadku na ekranie pojawią się wszystkie hasła Twoich użytkowników. Druga sprawa jest taka, że te hasła nie są zahaszowane (poczytaj trochę o md5 i sha1), więc nawet jeśli komuś udałoby się ukraść te hasła z pliku passwords.txt to i tak musiałby się trochę namęczyć aby te hasła odzyskać. Powtarzam jest to bardzo duża luka więc tu na pewno musisz ją poprawić

To tyle wstępem, jeszcze trochę musisz się pouczyć, ale popieram własną inicjatywę przy pisaniu kodu.

Pozdrawiam

Cytat(Cezar708 @ 21.01.2008, 09:39:24 )

1. restricted.php, linia 11
masz tam teraz if(!isset($otwarcia)), a nie wiadomo skąd wzięła się zmienna $otwarcia.

Ona powstaje właśnie w tym miejscu - jeśli nie istniała wcześniej - czyli przy pierwszej odsłonie tego pliku.
Co prawda napisałem, że decyduje ona o tym czy trzeba się zalogować, jednak nawet jeśli nie trzeba ($otwarcia >0) to funkcja sprawdz() i tak jest każdorazowo odpalana (secur.php, ln 50). Więc chyba nie da się tego wykorzystać jako luka w bezpieczeństwie.

Cytat(Cezar708 @ 21.01.2008, 09:39:24 )

2. secur.php linia 6
include("passwords.txt"); - tu baardzo duża luka... co się stanie jeśli w przeglądarce wpiszesz: http://<domena>/passwords.txt? Założę się, że w tym przypadku na ekranie pojawią się wszystkie hasła Twoich użytkowników.

Racja, ale... Skąd byś wiedział jak się nazywa ten plik? Powyższą nazwę można jeszcze zagadywać w ciemno, ale oczywiście nazwałem go inaczej (tu wpisałem tak z 2 powodów: aby kod był czytelniejszy oraz jakby ktoś wyniuchał gdzie ten kod jest wykorzystany :] ). Plik może być też umieszczony w podkatalogu. Z kolei gdyby ktoś mógł listować katalog na serwerze to nie musi szukać haseł tylko wejść wprost w pliki do których powyższe hasła dostępu bronią.
BTW Da się przeskoczyć brak praw do listowania katalogu (na na założeniu, że nie, opiera się mój skrypt)?

pozdrawiam
Swierszcz

Podaj mi adres strony gdzie takie coś wykorzystałeś a coś ci pokarze ;P (wyslij na pw)