Pomoc - Szukaj - U¿ytkownicy - Kalendarz
Pe³na wersja: [skrypt] Logowanie
Forum PHP.pl > Inne > Oceny
sebekzosw
19.08.2009, 23:07:12
Witam!

Mam taki skrypt logowania opartego o sesje - moje pytanie brzmi: Czy taki skrypt jest bezpieczny?:

[PHP] pobierz, plaintext 
  1. <?php
  2.     function DodawanieDoBazy($ciag) {
  3.         $wynik = stripslashes(strip_tags(addslashes(trim($ciag))));
  4.         $wynik = str_replace(array("\"", "'", "\\", '\"', "\'", "<", ">", "&nbsp;"), array("&quot;", "'", "\", "&quot;", "'", "&lt;", "&gt;", " "), $wynik);
  5.         $wynik = preg_replace(array("/\=/","/\#/","/\sOR\s/"), "", $wynik);
  6.         return $wynik;
  7.     }
  8.  
  9.  
  10.     if(isset($_POST["logowanie"]) && isset($_POST['login']) && isset($_POST['login'])) {
  11.         $logon_failure = '';
  12.         if(empty($_POST["login"])) { $logon_failure .= "Wpisz login<br />"; }
  13.         if(empty($_POST["password"])) { $logon_failure .= "Wpisz has³o<br />"; }
  14.         $login = DodawanieDoBazy($_POST["login"]);
  15.         $password = md5($_POST["password"]);
  16.  
  17.         if(empty($logon_failure)) {
  18.             if(!mysql_fetch_row(mysql_query("SELECT * FROM users WHERE login='".$login."' AND pass='".$password."'"))) {
  19.                 $logon_failure .= "Nieprawid³owy login lub has³o";
  20.             } elseif(!mysql_fetch_row(mysql_query("SELECT * FROM users WHERE login='".$login."' AND activity='1'"))) {
  21.                 $logon_failure .= "Konto jest nieaktywne";
  22.             }
  23.  
  24.             if(empty($logon_failure)) {
  25.                 $_SESSION["login"] = $login;
  26.                  $_SESSION['ip'] = $_SERVER['REMOTE_ADDR'];
  27.                  $_SESSION['browser'] = $_SERVER['HTTP_USER_AGENT'];
  28.             } else {
  29.                 echo $logon_failure;
  30.             }
  31.         } else {
  32.             echo $logon_failure;
  33.         }
  34.     }
  35.     echo "<div id=\"menu\">";
  36.     if(UZYTKOWNIK) {
  37.         echo "Zalogowany jako ".$_SESSION["login"];
  38.     } else {
  39.  
  40.  
  41. ?>
  42.     <form action="" method="post" id="logowanie">
  43.         <fieldset>
  44.             <dl>
  45.                 <dt>Login</dt>
  46.                  <dd><input type="text" name="login" id="login" value="Login" onblur="if(this.value=='') this.value='Login';" onfocus="if(this.value=='Login') this.value='';" /></dd>
  47.  
  48.                 <dt>Has³o:</dt>
  49.                  <dd><input type="password" name="password" id="password" value="Has³o" onblur="if(this.value=='') this.value='Has³o';" onfocus="if(this.value=='Has³o') this.value='';" /></dd>
  50.             </dl>
  51.         </fieldset>
  52.         <div style="float:right;"><input type="submit" name="logowanie" value="Zaloguj siê" onclick="WyslijFormularz('logowanie'); return false;" /></div>
  53.     </form>
  54.     <img src="http://test.php.pl/www/gfx/login_loading.gif" id="ladowanie" alt="" style="display:none;float:left;" />
  55.     <div id="wynik"></div>
  56.  
  57.     <p><a href="/rejestracja/">Rejestracja</a>
  58.         <br />
  59.         <a href="/przypomnij/">Przypomnij has³o</a>
  60.  
  61.  
  62.  <?
  63.  }
  64.  ?>
  65.  </div>
[PHP] pobierz, plaintext


Wszystkie sugestie na temat polepszenia mile widziane smile.gif
Quantum
20.08.2009, 06:00:49
Cytat(sebekzosw)
[PHP] pobierz, plaintext 
  1. ie"]) && isset($_POST['login']) && isset($_POST['login'])
[PHP] pobierz, plaintext


w jakim celu sprawdzasz 2 razy ? biggrin.gif dodatkowo dziwna nazwa funkcji, DodawanieDoBazy, lepiej by³oby FiltrujZapytanie, choæ i tak jestem zwolennikiem nazewnictwa ang. u¿ywaj http://www.php.net/mysql_real_escape_string.
Dlaczego raz u¿ywasz ' a raz " ? zast±p cudzys³ów apostrofem, zwiêksza to szybko¶æ dzia³ania skryptu.
Co do bezpieczeñstwa.. MD5 ? ta funkcja skrótu teoretycznie w nowych aplikacjach pojawiaæ siê nie powinna, zast±p j± SHA-1. Od siebie dodam, ¿e lepiej napisa³by¶ to obiektowo, proceduralne - gorzej siê czyta, po 2 tygodniach przerwy nie bêdziesz wiedzia³ jak on nawet dzia³a³, OOP daje wiêksze mo¿liwo¶ci, na ten przyk³ad PDO - nie musia³by¶ martwiæ siê o bezpieczeñstwo, zapytania s± tam filtrowane. Dodatkowo rozbudowa takich skryptów jest o wiele szybsza i prostsza.

[PHP] pobierz, plaintext 
  1. if(!mysql_fetch_row(mysql_query("SELECT * FROM users WHERE login='".$login."' AND pass='".$password."'"))) {
[PHP] pobierz, plaintext


zast±p tym, bêdzie wydajniej

[SQL] pobierz, plaintext 
  1. SELECT count(*) FROM users WHERE login='".$login."' AND pass='".$password."'
[SQL] pobierz, plaintext
sebekzosw
20.08.2009, 08:30:28
Poprawi³em ju¿ to co mówi³em, tylko nie wiem gdzie zastosowaæ mysql_real_escape_stringquestionmark.gif I zamiast czego to u¿yæ smile.gif

I dobrze zrobi³em z tym??:

[PHP] pobierz, plaintext 
  1. <?php
  2. if(!mysql_result(mysql_query("SELECT count(*) FROM users WHERE login='".$login."' AND pass='".$password."'"), 0)) {
  3.         		$logon_failure .= "Nieprawid³owy login lub has³o";
  4.         	}
  5. ?>
[PHP] pobierz, plaintext
Quantum
21.08.2009, 13:26:57
1. wystarczy zajrzeæ do manuala, przyk³ad 3, widaæ czarno na bia³ym jak u¿ywaæ i do czego mysql_real_escape_string s³u¿y
2. sprawd¼ czy dzia³a winksmiley.jpg
drake88
23.08.2009, 15:19:17
Cytat(sebekzosw @ 20.08.2009, 09:30:28 ) *
I dobrze zrobi³em z tym??:

[PHP] pobierz, plaintext
  1. <?php
  2. if(!mysql_result(mysql_query("SELECT count(*) FROM users WHERE login='".$login."' AND pass='".$password."'"), 0)) {
  3. $logon_failure .= "Nieprawid³owy login lub has³o";
  4. }
  5. ?>
[PHP] pobierz, plaintext


Jak najbardziej dobrze.
sebekzosw
24.08.2009, 14:48:32
Trochê poprawi³em ca³e logowanie dodaj±c now± opcjê, co s±dzicie teraz o takim skrypcie:

[PHP] pobierz, plaintext 
  1. <?php
  2.    	session_start();
  3.  
  4.    	function DodawanieDoBazy($ciag) {
  5.     	$wynik = stripslashes(strip_tags(addslashes(trim($ciag))));
  6. 		$wynik = str_replace(array("\"", "'", "\\", '\"', "\'", "<", ">", " "), array(""", "'", "\", """, "'", "<", ">", " "), $wynik);
  7.         $wynik = preg_replace(array("/\=/","/\#/","/\sOR\s/"), "", $wynik);
  8.         return $wynik;
  9.     }
  10.  
  11. 	if(isset($_POST["logowanie"]) && isset($_POST["login"])) {
  12. 		$logon_failure = "";
  13. 		if(empty($_POST["login"]) OR $_POST["login"] == "Login") { $logon_failure .= "Wpisz login<br />"; }
  14. 		if(empty($_POST["password"]) OR $_POST["password"] == "Has³o") { $logon_failure .= "Wpisz has³o<br />"; }
  15.         $login = DodawanieDoBazy($_POST["login"]);
  16.         $password = sha1($_POST["password"]);
  17.  
  18.         if(empty($logon_failure)) {
  19.         	if(!mysql_result(mysql_query("SELECT count(*) FROM users WHERE login='".$login."' AND pass='".$password."'"), 0)) {
  20.         		mysql_query("UPDATE users SET login_failed=login_failed+1, date_last_failed_login='".time()."' WHERE login='".$login."' LIMIT 1"); //Dodawanie +1 do obecnej liczby b³êdnych logowañ
  21.         		$zapytanie = mysql_fetch_assoc(mysql_query("SELECT * FROM users WHERE login='".$login."'"));
  22.         		if($zapytanie["login_failed"] > 3) {
  23.         			mysql_query("UPDATE users SET activity='0' WHERE login='".$login."' LIMIT 1"); //Deaktywacja konta po 3 b³êdnych próbach logowania
  24.         			$logon_failure = "Konto zosta³o zdeaktywowane, poniewa¿ liczba prób logowania by³a wiêksza ni¿ 3. Aby aktywowaæ konto, kliknij <a href=\"\">tu</a>";
  25.         		} else {
  26.         			$logon_failure .= "Nieprawid³owy login lub has³o";
  27.         		}
  28.         	} elseif(!mysql_result(mysql_query("SELECT count(*) FROM users WHERE login='".$login."' AND activity='1'"), 0)) { //Sprawdzamy czy konto jest aktywne
  29.         		$logon_failure .= "Konto jest nieaktywne";
  30.         	}
  31.  
  32.         	if(empty($logon_failure)) {
  33.         		$zapytanie = mysql_fetch_assoc(mysql_query("SELECT * FROM users WHERE login='".$login."'"));
  34.             	mysql_query("UPDATE users SET last_login='".$zapytanie["penultimate_login"]."', penultimate_login='".time()."' WHERE id='".$zapytanie["id"]."' LIMIT 1"); //Aktualizacja danych o ostatnim logowaniu
  35. 				mysql_query("UPDATE users SET login_failed='0' WHERE login='".$login."' LIMIT 1"); //Resetowanie liczby b³êdnych logowañ
  36. 				$_SESSION["login"] = $login;
  37. 	 			$_SESSION["ip"] = $_SERVER["REMOTE_ADDR"];
  38.  				$_SESSION["browser"] = $_SERVER["HTTP_USER_AGENT"];
  39.  			} else {
  40.         		echo $logon_failure;
  41.         	}
  42.         } else {
  43. 			echo $logon_failure;
  44. 		}
  45. 	}
  46.     echo "<div id=\"menu\">";
  47. 	if(UZYTKOWNIK) {
  48. 		echo "Zalogowany jako ".$_SESSION["login"];
  49. 	} else {
  50.  
  51.  
  52. ?>
  53. 	<form action="" method="post" id="logowanie">
  54. 		<fieldset>
  55. 	    	<dl>
  56. 	    		<dt>Login</dt>
  57. 	     		<dd><input type="text" name="login" id="login" value="Login" onblur="if(this.value=='') this.value='Login';" onfocus="if(this.value=='Login') this.value='';" /></dd>
  58.  
  59. 	    		<dt>Has³o:</dt>
  60. 	     		<dd><input type="password" name="password" id="password" value="Has³o" onblur="if(this.value=='') this.value='Has³o';" onfocus="if(this.value=='Has³o') this.value='';" /></dd>
  61. 	    	</dl>
  62. 		</fieldset>
  63. 		<div style="float:right;"><input type="submit" name="logowanie" value="Zaloguj siê" onclick="sWyslijFormularz('logowanie'); return false;" /></div>
  64. 	</form>
  65. 	<img src="http://test.php.pl/www/gfx/login_loading.gif" id="ladowanie" alt="" style="display:none;float:left;" />
  66. 	<div id="wynik"></div>
  67.  
  68. 	<p><a href="/rejestracja/">Rejestracja</a>
  69. 		<br />
  70.         <a href="/przypomnij/">Przypomnij has³o</a>
  71.  
  72.  
  73.  <?
  74.  }
  75.  ?>
  76.  </div>
[PHP] pobierz, plaintext
drake88
26.08.2009, 21:29:19
Mo¿e najpierw powiedz, jak± opcjê doda³e¶? ^^
sebekzosw
27.08.2009, 13:24:20
Deaktywacja konta po 3 b³êdnych próbach logowania
Fifi209
27.08.2009, 13:40:19 
[PHP] pobierz, plaintext 
  1. $wynik = stripslashes(strip_tags(addslashes(trim($ciag))));
  2. $wynik = str_replace(array("\"", "'", "\\", '\"', "\'", "<", ">", " "), array(""", "'", "\", """, "'", "<", ">", " "), $wynik);
  3. $wynik = preg_replace(array("/\=/","/\#/","/\sOR\s/"), "", $wynik);
[PHP] pobierz, plaintext


Jaki w tym sens widzisz?

addslashes doda \
strislashes usunie \
str_replace jest nie potrzebny tak samo preg_replace

U¿yj po prostu mysql_real_escape_string?

I jeszcze to:
[PHP] pobierz, plaintext 
  1. if(UZYTKOWNIK) {
[PHP] pobierz, plaintext


Gdzie definiujesz t± sta³±?
l0ud
27.08.2009, 13:48:02
Cze¶æ

Ca³a funkcja DodawanieDoBazy() jest zupe³nie niepotrzebna, najlepszym i najprostszym zabezpieczeniem przed SQL injection oraz XSS jest przepuszczenie ci±gu do dodania do bazy przez mysql_real_escape_string, a nastêpnie - przy wy¶wietlaniu (a nie dodaniu do bazy) htmlspecialchars.

W tym kodzie zamiast
[PHP] pobierz, plaintext 
  1. $login = DodawanieDoBazy($_POST["login"]);
[PHP] pobierz, plaintext


U¿yj po prostu
[PHP] pobierz, plaintext 
  1. $login = mysql_real_escape_string($_POST["login"]);
[PHP] pobierz, plaintext


(Albo przepuszczaj login przez mysql_real_escape_string bezpo¶rednio w zapytaniu)
Fifi209
27.08.2009, 13:50:19
Cytat(l0ud @ 27.08.2009, 13:48:02 ) *
Cze¶æ

Ca³a funkcja DodawanieDoBazy() jest zupe³nie niepotrzebna, najlepszym i najprostszym zabezpieczeniem przed SQL injection oraz XSS jest przepuszczenie ci±gu do dodania do bazy przez mysql_real_escape_string, a nastêpnie - przy wy¶wietlaniu (a nie dodaniu do bazy) htmlspecialchars.

W tym kodzie zamiast
[PHP] pobierz, plaintext 
  1. $login = DodawanieDoBazy($_POST["login"]);
[PHP] pobierz, plaintext


U¿yj po prostu
[PHP] pobierz, plaintext 
  1. $login = mysql_real_escape_string($_POST["login"]);
[PHP] pobierz, plaintext


(Albo przepuszczaj login przez mysql_real_escape_string bezpo¶rednio w zapytaniu)

Po prostu to sobie daruj takie posty.

Napisa³e¶ to co ja...(tylko w innych s³owach)
l0ud
27.08.2009, 14:06:29
Cytat
Po prostu to sobie daruj takie posty.

Napisa³e¶ to co ja...(tylko w innych s³owach)


Mhm... a nie pomy¶la³e¶ ¿e mog³em po prostu d³u¿ej formu³owaæ tego posta? Poza tym nie wspomnia³e¶ o htmlspecialchars której dzia³anie czyni równie¿ jego funkcja.
Fifi209
27.08.2009, 14:09:12
Cytat(l0ud @ 27.08.2009, 14:06:29 ) *
Mhm... a nie pomy¶la³e¶ ¿e mog³em po prostu d³u¿ej formu³owaæ tego posta? Poza tym nie wspomnia³e¶ o htmlspecialchars której dzia³anie czyni równie¿ jego funkcja.


O 7 minut ?

htmlspecialchars zamienia na encje - jego funkcja tego nie robi... Wiêc o czym tu mowa.
l0ud
27.08.2009, 14:23:49
Tak, o 7 minut. Co do htmlspecialchars, robi³a to poprzednia wersja funkcji, a na ni± patrza³em (mój b³ad). Niemniej w takiej formie jak teraz tym bardziej nale¿a³oby o tym wspomnieæ, bo wyrzucenie zmiennej bez filtracji to dziura xss.
btw. w³a¶nie wywo³a³e¶ bezsensown± dyskusjê na 5 postów tongue.gif Du¿o lepiej by by³o, jakby¶ w takich sprawach po prostu napisa³ na PW.
Fifi209
27.08.2009, 14:52:49
Dyskusja jest te¿ na temat. winksmiley.jpg

Napisa³e¶, przy wyrzucaniu z bazy u¿ywaæ htmlspecialchars tylko powiedz mi jaki ma sens ci±g³e obci±¿anie parsera jak mo¿na to zrobiæ raz przy zapisie do bazy?
l0ud
27.08.2009, 16:05:07
Daje mo¿liwo¶æ bezproblemowej edycji i wy¶wietlenia danych w wersji niezmodyfikowanej - ³atwo chocia¿by wygenerowaæ listê u¿ytkowników do pliku .txt. Poza tym takie zachowanie to przecie¿ standard tongue.gif
klocu
28.08.2009, 09:18:11
A tak w³a¶ciwie to czy jest sens bombardowaæ bazê tymi wszystkimi zapytaniami?
Nie lepiej by³oby na pocz±tku wyci±gaæ usera po loginie i potem sprawdzaæ jego parametry zgodnie z Twoimi za³o¿eniami?
Bo tak mo¿esz maksymalnie zmie¶ciæ siê w: 1 SELECT + 1 UPDATE (przy b³êdnym zalogowaniu) [dobrze my¶lê?]

Jednym s³owem wartoby trochê zoptymalizowaæ skrypt. Ale droga jest chyba dobra, tak s±dzê.
phpion
28.08.2009, 09:24:55
Cytat(klocu @ 28.08.2009, 10:18:11 ) *
Nie lepiej by³oby na pocz±tku wyci±gaæ usera po loginie i potem sprawdzaæ jego parametry zgodnie z Twoimi za³o¿eniami?

S³uszna uwaga!

Cytat(sebekzosw @ 27.08.2009, 14:24:20 ) *
Deaktywacja konta po 3 b³êdnych próbach logowania

Bardzo niebezpieczna opcja... wystarczy jeden z³o¶liwy u¿ytkownik aby poblokowaæ Ci innych u¿ytkowników.
sebekzosw
29.08.2009, 12:46:37
Demo (Login: demo , Has³o: demo) - sprawd¼cie bezpieczeñstwo smile.gif

a oto kod:

[PHP] pobierz, plaintext 
  1. <?php
  2. 	if(isset($_POST["logowanie"]) && isset($_POST["login"])) {
  3. 		$logon_failure = "";
  4. 		if(empty($_POST["login"]) OR $_POST["login"] == "Login") { $logon_failure .= "Wpisz login<br />"; }
  5. 		if(empty($_POST["password"]) OR $_POST["password"] == "Has³o") { $logon_failure .= "Wpisz has³o<br />"; }
  6.         $login = mysql_real_escape_string($_POST["login"]);
  7.         $password = sha1($_POST["password"]);
  8.  
  9.         $users = mysql_fetch_assoc(mysql_query("SELECT * FROM users WHERE login='".$login."'"));
  10.  
  11.         if(empty($logon_failure)) {
  12.         	if($users["pass"] != $password) {
  13.         		if($ustawienia["off_account"] == 1) {
  14. 	        		if($users["login_failed"] >= 3) {
  15. 	        			mysql_query("UPDATE users SET activity='0' WHERE login='".$login."' LIMIT 1"); //Deaktywacja konta po 3 b³êdnych próbach logowania
  16. 	        			$logon_failure = "Konto zosta³o zdeaktywowane, poniewa¿ liczba prób logowania by³a wiêksza ni¿ 3. Aby aktywowaæ konto, kliknij <a href=\"\">tu</a>";
  17. 	        		} else {
  18. 	        			mysql_query("UPDATE users SET login_failed=login_failed+1, date_last_failed_login='".time()."' WHERE login='".$login."' LIMIT 1"); //Dodawanie +1 do obecnej liczby b³êdnych logowañ
  19. 	        			$logon_failure .= "Nieprawid³owy login lub has³o";
  20. 	        		}
  21. 	       		} else {
  22. 	       			$logon_failure .= "Nieprawid³owy login lub has³o";
  23. 	       		}
  24.         	} elseif($users["activity"] == 0) {
  25.         		$logon_failure .= "Konto jest nieaktywne";
  26.         	}
  27.  
  28.         	if(empty($logon_failure)) {
  29.             	mysql_query("UPDATE users SET last_login='".$users["penultimate_login"]."', penultimate_login='".time()."', login_failed='0' WHERE id='".$users["id"]."' LIMIT 1"); //Aktualizacja danych o ostatnim logowaniu
  30. 				$_SESSION["login"] = $login;
  31. 	 			$_SESSION["ip"] = $_SERVER["REMOTE_ADDR"];
  32.  				$_SESSION["browser"] = $_SERVER["HTTP_USER_AGENT"];
  33.  			} else {
  34.         		echo $logon_failure;
  35.         	}
  36.         } else {
  37. 			echo $logon_failure;
  38. 		}
  39. 	}
  40.     echo "<div id=\"menu\">";
  41. 	if(UZYTKOWNIK) {
  42. 		echo "Zalogowany jako ".$_SESSION["login"];
  43. 	} else {
  44.  
  45.  
  46. ?>
  47. 	<form action="" method="post" id="logowanie">
  48. 		<fieldset>
  49. 	    	<dl>
  50. 	    		<dt>Login</dt>
  51. 	     		<dd><input type="text" name="login" id="login" value="Login" onblur="if(this.value=='') this.value='Login';" onfocus="if(this.value=='Login') this.value='';" /></dd>
  52.  
  53. 	    		<dt>Has³o:</dt>
  54. 	     		<dd><input type="password" name="password" id="password" value="Has³o" onblur="if(this.value=='') this.value='Has³o';" onfocus="if(this.value=='Has³o') this.value='';" /></dd>
  55. 	    	</dl>
  56. 		</fieldset>
  57. 		<div style="float:right;"><input type="submit" name="logowanie" value="Zaloguj siê" onclick="sWyslijFormularz('logowanie'); return false;" /></div>
  58. 	</form>
  59. 	<img src="http://test.php.pl/www/gfx/login_loading.gif" id="ladowanie" alt="" style="display:none;float:left;" />
  60. 	<div id="wynik"></div>
  61.  
  62. 	<p><a href="/rejestracja/">Rejestracja</a>
  63. 		<br />
  64.         <a href="/przypomnij/">Przypomnij has³o</a>
  65.  
  66.  
  67.  <?
  68.  }
  69.  ?>
  70.  </div>
[PHP] pobierz, plaintext


Ograniczy³em siê do 1 SELECT, ale nie mam pomys³u na 1 UPDATE. Jakie¶ propozycje polepszenia?
To jest wersja lo-fi g³ównej zawarto¶ci. Aby zobaczyæ pe³n± wersjê z wiêksz± zawarto¶ci±, obrazkami i formatowaniem proszê kliknij tutaj.
Invision Power Board © 2001-2025 Invision Power Services, Inc.