Witaj Gościu! ( Zaloguj | Rejestruj )

Forum PHP.pl

2 Stron V   1 2 >  
Reply to this topicStart new topic
> $_SESSION i Uwierzytelnianie - pytanie
MGreg
post 4.06.2008, 17:32:36
Post #1





Grupa: Zarejestrowani
Postów: 24
Pomógł: 0
Dołączył: 18.09.2007

Ostrzeżenie: (0%)
-----


Witam.
Męczę się nad bezpiecznym systemem uwierzytelniania i proszę ocenić, czy takie coś będzie bezpieczne:
  1. <?php
  2.  
  3. if(isset($_POST['login']) and isset($_POST['haslo']) and $_GET['action']=='zaloguj')
  4. {
  5. $query = mysql_query("select * from `users` where `login`='".mysql_real_escape_string($_POST['login'])."' and `haslo`='".mysql_real_escape_string(md5($_POST['haslo'])).'"');
  6. if(mysql_num_rows($query)>0)
  7.  {
  8.  $_SESSION['logged'] = 1;
  9.  $_SESSION['username'] = $_POST['login'];
  10.  }
  11. else
  12. {
  13. $_SESSION['logged']=0;
  14. {
  15. }
  16.  
  17.  if($_SESSION['logged']==1)
  18. {
  19.  /* JAKIŚ TAM PANEL ADMINISTRACYJNY */
  20. }
  21.  
  22. ?>


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

Ten post edytował MGreg 4.06.2008, 17:35:26
Go to the top of the page
+Quote Post
marcio
post 4.06.2008, 17:38:11
Post #2





Grupa: Zarejestrowani
Postów: 2 291
Pomógł: 156
Dołączył: 23.09.2007
Skąd: ITALY-MILAN

Ostrzeżenie: (10%)
X----


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


--------------------
Zainteresowania: XML | PHP | MY(SQL)| C# for .NET | PYTHON
http://code.google.com/p/form-builider/
Moj blog
Go to the top of the page
+Quote Post
MGreg
post 4.06.2008, 17:52:21
Post #3





Grupa: Zarejestrowani
Postów: 24
Pomógł: 0
Dołączył: 18.09.2007

Ostrzeżenie: (0%)
-----


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 smile.gif
Go to the top of the page
+Quote Post
pyro
post 4.06.2008, 17:58:16
Post #4





Grupa: Zarejestrowani
Postów: 2 148
Pomógł: 230
Dołączył: 26.03.2008

Ostrzeżenie: (0%)
-----


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


--------------------
ET LINGUA EIUS LOQUETUR IUDICIUM
Go to the top of the page
+Quote Post
Shili
post 4.06.2008, 18:10:55
Post #5





Grupa: Zarejestrowani
Postów: 1 085
Pomógł: 231
Dołączył: 12.05.2008

Ostrzeżenie: (0%)
-----


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?

Ten post edytował Shili 4.06.2008, 18:12:24
Go to the top of the page
+Quote Post
MGreg
post 4.06.2008, 18:15:58
Post #6





Grupa: Zarejestrowani
Postów: 24
Pomógł: 0
Dołączył: 18.09.2007

Ostrzeżenie: (0%)
-----


Cytat(Shili @ 4.06.2008, 19:10:55 ) *
@up
A gdzie tu masz wypisywanie jakichkolwiek danych wprowadzonych przez użytkownika?


Nie umieszczałem formularza, bo chodzi mi głównie o kod php smile.gif Ale jak to ma się na coś zdać to dodałbym tam np.
  1. <form action="login.php?action=login" method=post>
  2. login <input type=text name=login><br>
  3. hasło <input type=password name=haslo<br>
  4. <input type=submit value=zaloguj>
  5. </form>

smile.gif

Ten post edytował MGreg 4.06.2008, 18:17:14
Go to the top of the page
+Quote Post
Shili
post 4.06.2008, 18:28:19
Post #7





Grupa: Zarejestrowani
Postów: 1 085
Pomógł: 231
Dołączył: 12.05.2008

Ostrzeżenie: (0%)
-----


Nie nie, to było do @pyro. Powinnam była wyraźniej zaadresować winksmiley.jpg
Go to the top of the page
+Quote Post
pyro
post 4.06.2008, 20:15:32
Post #8





Grupa: Zarejestrowani
Postów: 2 148
Pomógł: 230
Dołączył: 26.03.2008

Ostrzeżenie: (0%)
-----


Cytat(Shili @ 4.06.2008, 19:10:55 ) *
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?


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


--------------------
ET LINGUA EIUS LOQUETUR IUDICIUM
Go to the top of the page
+Quote Post
Shili
post 4.06.2008, 20:27:34
Post #9





Grupa: Zarejestrowani
Postów: 1 085
Pomógł: 231
Dołączył: 12.05.2008

Ostrzeżenie: (0%)
-----


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ć winksmiley.jpg

Ten post edytował Shili 4.06.2008, 20:28:08
Go to the top of the page
+Quote Post
MGreg
post 4.06.2008, 20:54:44
Post #10





Grupa: Zarejestrowani
Postów: 24
Pomógł: 0
Dołączył: 18.09.2007

Ostrzeżenie: (0%)
-----


Co do XSS stosuję strip_tags(); i chyba to mi wystarczy smile.gif Jutro wstawię poprawioną wersję i będziemy dyskutować dalej smile.gif
Go to the top of the page
+Quote Post
LonelyKnight
post 4.06.2008, 21:02:40
Post #11





Grupa: Zarejestrowani
Postów: 240
Pomógł: 13
Dołączył: 1.06.2007
Skąd: Wrocław

Ostrzeżenie: (0%)
-----


Cytat(pyro @ 4.06.2008, 21:15:32 ) *
No tak... to co pokazał autor to jest pewnie cały skrypt, prawda Shili?



To jeszcze powiedz autorowi jak powinien zabezpieczyć serwer bo przecież gdzieś to musi uruchomić sciana.gif 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.

Ten post edytował LonelyKnight 5.06.2008, 09:36:37


--------------------
Good programming is 99% sweat and 1% coffee.
Make it idiot proof and someone will make a better idiot...
Go to the top of the page
+Quote Post
MGreg
post 5.06.2008, 08:25:43
Post #12





Grupa: Zarejestrowani
Postów: 24
Pomógł: 0
Dołączył: 18.09.2007

Ostrzeżenie: (0%)
-----


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.

  1. <?php
  2. include('config.php');
  3. pol_mysql();
  4.  
  5. if($_GET['action']=='logout')
  6. {
  7. if($_SESSION['logged']==1)
  8. {
  9. $_SESSION['logged']=0;
  10. echo "<b>zostałeś wylogowany<b>, jeśli nie zostaniesz przeniesiony na stronę logowania kliknij <a href=login.php>tutaj</a>";
  11. header("refresh: 5,login.php");
  12. }
  13. else
  14. {
  15. echo "nie byłeś zalogowany!";
  16. }
  17. }
  18.  
  19. if(!isset($_SESSION['logged'])){$_SESSION['logged']=0;}
  20.  
  21. if(isset($_POST['login']) and isset($_POST['haslo']))
  22.  {  
  23.  if(strlen($_POST['login'])<25 and strlen($_POST['haslo'])<25)
  24.  {  
  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");
  26.  
  27.  if(mysql_num_rows($query)===1)
  28.  {  
  29.  $_SESSION['logged'] = 1;  
  30.  }
  31.  else
  32. {
  33. echo "Błędny login lub hasło";
  34. $_SESSION['logged']= 0;
  35. }
  36.  }  
  37.  else
  38. {
  39. echo "Zbyt długi login lub hasło";
  40. }  
  41.  }
  42.  
  43.  if($_SESSION['logged']==1)
  44.  {  
  45.  echo "Jakiś tam panel admina <a href=login.php?action=logout>Wyloguj</a>";  
  46.  }  
  47.  
  48.  
  49.  
  50. if($_SESSION['logged']==0 and $_GET['action']!=='logout')
  51. {
  52. ?>
  53. <form action="login.php" method=post>
  54. login <input type=text name=login><br>
  55. hasło <input type=password name=haslo><br>
  56. <input type=submit value=zaloguj>
  57. </form>
  58. <?
  59. }
  60.  
  61. ?>


Czy teraz jest lepiej?

Ten post edytował MGreg 5.06.2008, 10:27:15
Go to the top of the page
+Quote Post
Cotter
post 5.06.2008, 10:22:53
Post #13





Grupa: Zarejestrowani
Postów: 57
Pomógł: 12
Dołączył: 6.01.2008
Skąd: Wrocław

Ostrzeżenie: (0%)
-----


Cytat(LonelyKnight @ 4.06.2008, 22:02:40 ) *
5. Nie trzymaj w sesji nazwy użytkownika tylko np. wspomniany już losowy identyfikator...


Dlaczego trzymanie nazwy użytkownika w sesji może być niebezpieczne?
Go to the top of the page
+Quote Post
LonelyKnight
post 5.06.2008, 11:27:19
Post #14





Grupa: Zarejestrowani
Postów: 240
Pomógł: 13
Dołączył: 1.06.2007
Skąd: Wrocław

Ostrzeżenie: (0%)
-----


Cytat(Cotter @ 5.06.2008, 11:22:53 ) *
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.


--------------------
Good programming is 99% sweat and 1% coffee.
Make it idiot proof and someone will make a better idiot...
Go to the top of the page
+Quote Post
MGreg
post 5.06.2008, 11:49:12
Post #15





Grupa: Zarejestrowani
Postów: 24
Pomógł: 0
Dołączył: 18.09.2007

Ostrzeżenie: (0%)
-----


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ć?
Go to the top of the page
+Quote Post
LonelyKnight
post 5.06.2008, 12:33:43
Post #16





Grupa: Zarejestrowani
Postów: 240
Pomógł: 13
Dołączył: 1.06.2007
Skąd: Wrocław

Ostrzeżenie: (0%)
-----


Może być.


session_regenerate_id(true) zamist session_regenerate_id().

Ten post edytował LonelyKnight 5.06.2008, 12:34:06


--------------------
Good programming is 99% sweat and 1% coffee.
Make it idiot proof and someone will make a better idiot...
Go to the top of the page
+Quote Post
pyro
post 5.06.2008, 15:49:34
Post #17





Grupa: Zarejestrowani
Postów: 2 148
Pomógł: 230
Dołączył: 26.03.2008

Ostrzeżenie: (0%)
-----


Cytat(LonelyKnight @ 4.06.2008, 22:02:40 ) *
To jeszcze powiedz autorowi jak powinien zabezpieczyć serwer bo przecież gdzieś to musi uruchomić sciana.gif Autor pisze o systemie autoryzacji a Ty opowiadasz o XSS jakby to miało cokolwiek wspólnego.


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

Cytat(LonelyKnight @ 4.06.2008, 22:02:40 ) *
1. Nie używaj Select *...

Niby dlaczego nie?
Cytat(LonelyKnight @ 4.06.2008, 22:02:40 ) *
2. Dodaj LIMIT do zapytania

Co to ma dać? Dłuższe zapytanie?
Cytat(LonelyKnight @ 4.06.2008, 22:02:40 ) *
3. Nie sprawdzaj mysql_num_rows($query)>0 tylko mysql_num_rows($query)===1 -> zalogowany

W przypadku tego kawałka kodu co to za różnica? Lecz powinno się też sprawdzać czy użytkownik już istnieje.
Cytat(LonelyKnight @ 4.06.2008, 22:02:40 ) *
5. Nie trzymaj w sesji nazwy użytkownika tylko np. wspomniany już losowy identyfikator...

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


Cytat
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.


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


--------------------
ET LINGUA EIUS LOQUETUR IUDICIUM
Go to the top of the page
+Quote Post
wlamywacz
post 5.06.2008, 16:55:39
Post #18





Grupa: Zarejestrowani
Postów: 535
Pomógł: 27
Dołączył: 3.05.2005

Ostrzeżenie: (20%)
X----


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
Go to the top of the page
+Quote Post
pyro
post 5.06.2008, 18:44:24
Post #19





Grupa: Zarejestrowani
Postów: 2 148
Pomógł: 230
Dołączył: 26.03.2008

Ostrzeżenie: (0%)
-----


Cytat(wlamywacz @ 5.06.2008, 17:55:39 ) *
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.


Czy chociaż sam rozumiesz co tu napisałeś ^^?
Cytat(wlamywacz @ 5.06.2008, 17:55:39 ) *
Limit to samo w razie ataku pobierze jednego usera a nie całą tablice userów.

Co to za różnica, tak czy inaczej mozna pobrac całą tablicę userów.
Cytat(wlamywacz @ 5.06.2008, 17:55:39 ) *
A co do sesji to lepiej dmuchać na zimne


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

Cytat
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ć?


Ten post edytował pyro 5.06.2008, 18:45:21


--------------------
ET LINGUA EIUS LOQUETUR IUDICIUM
Go to the top of the page
+Quote Post
Shili
post 5.06.2008, 20:25:36
Post #20





Grupa: Zarejestrowani
Postów: 1 085
Pomógł: 231
Dołączył: 12.05.2008

Ostrzeżenie: (0%)
-----


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 tongue.gif

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.
Go to the top of the page
+Quote Post

2 Stron V   1 2 >
Reply to this topicStart new topic
1 Użytkowników czyta ten temat (1 Gości i 0 Anonimowych użytkowników)
0 Zarejestrowanych:

 



RSS Wersja Lo-Fi Aktualny czas: 29.05.2020 - 11:18