Pomoc - Szukaj - Użytkownicy - Kalendarz
Pełna wersja: Rejestracja użytkowników
Forum PHP.pl > Forum > PHP
tadeurz
Napisałem sobie skrypt który zapisuje mi użytkowników do bazy danych.Zastanawiam się nad jego bezpieczeństwem moglibyście rzucić okiem ?

Proszę, bo naczytałem się w internecie: zawsze addslashed i htmlspecialchars(), moża też spotkać coś takiego :
$email = htmlspecialchars(stripslashes(strip_tags(trim($_POST["email"]))), ENT_QUOTES); <- tworzenie takiego czegoś dla hasła kompletnie go zmieni.

Przeglądałem i zastanawiałem się nad moim kodem wiele razy, według mnie nie ma żadnej luki.Ale nie daje mi to spokoju.

  1. if( $this->ajax ){
  2. $email = $_POST['email'];
  3. $password = $_POST['password'];
  4. $recaptcha = $_POST['recaptcha'];
  5. $challenge = $_POST['challenge'];
  6.  
  7. $recaptchaOK = false;
  8. if( !empty($recaptcha) && !empty($challenge) ){
  9. require_once('phpClass/recaptcha.php');
  10. $privatekey = '6Lfn8tsSAAAAALtc6Ny******************';
  11. $resp = recaptcha_check_answer ($privatekey,$_SERVER["REMOTE_ADDR"],$challenge,$recaptcha);
  12. $recaptchaOK = ( $resp->is_valid ) ? true : false ;
  13. }
  14.  
  15. $passwordOK = false;
  16. if( !empty($password) && strlen($password) < 40 ){
  17. require_once('phpClass/phpass.php');
  18. $hasher = new PasswordHash(8, TRUE);
  19. $password = $hasher->HashPassword($password);
  20. $passwordOK = ( strlen($password) >= 20 ) ? true : false ;
  21. }
  22.  
  23. $emailOK = false;
  24. if( !empty($email) ){
  25. $emailOK = ( preg_match('/^[0-9a-zA-Z_.-]+@[0-9a-zA-Z.-]+\.[a-zA-Z]{2,3}$/', $email) === 1 ) ? true : false ;
  26. $emailTaken = sql::qq('SELECT `id` FROM `user` WHERE `email`=? ','s',array($email));
  27. $emailTaken = ( $emailTaken ) ? true : false ;
  28. }
  29.  
  30. if( !$emailTaken && $emailOK && $passwordOK && $recaptchaOK ){
  31. $sqlR = sql::qq('INSERT INTO `user` (`email`,`password`) VALUES (?,?)','ss',array($email,$password));
  32. $this->body = array('status'=>true);
  33. }else{
  34. if( !$recaptchaOK ) $error['wrongRecaptcha']= $this->l('wrongRecaptcha');
  35. if( $emailTaken ) $error['takenEmail']= $this->l('takenEmail');
  36. if( !$emailOK ) $error['wrongEmail']= $this->l('wrongEmail');
  37. if( !$passwordOK ) $error['wrongPassword']=$this->l('wrongPassword');
  38. $this->body = array('status'=>false,'info'=>$error);
  39. }
  40. }else{
  41. error::NotAJAX();
  42. }


/---------------------------------
przepraszm za dziwne tab'y. Ten edytor coś wyzmieniał mi dlatego jest nieczytelnie.
timon27
Nie widzę potrzeby używania w haśle tych funkcji - i tak jest ono hashowane.

A co do reszty:

strlen($password) < 40
To nie ma żadnego sensu (w bazie i tak zapisujesz hasha, który zawsze ma taką samą długość).

strlen($password) >= 20

Przesadziłeś. Nawet do banku mam o wiele krótsze hasło.

Dodatkowo obsługa błądów. Rozumiem że jeśli $passwordOK==False, to użytkownik otrzyma informację:
"hasło jest za krótkie, lub hasło jest za długie, lub hasło nie zostało wpisane, lub inny błąd skryptowy."

Trochę to za mało szczegółowe i powinno być rozdzielone na 4 przypadki.

!*!
1. brakuje tu isset a to nie to samo co empty
2. wyrażenie do maila jest złe, ponieważ mail nie może mieć dużych znaków i domena.info nie przejdzie
3. zmień tabulacje na spacje w swoim edytorze a unikniesz rozjeżdżania kodu.
tadeurz
strlen($password) < 40 – wymaga aby hasło było mniejsze od 40 znaków, bo hash’owanie dłuższych hasłem może obciążać serwer.
Sunny Singh w swoim artykule podaje max 72, ja stwierdziłem że mam to sens i jest logiczne. Dlatego poszedłem dalej i ograniczyłem do 40.Nikt przy zdrowych zmysłach dłuższych hasłem nie stosuje, no chyba że ma złe intencje. Linika ma tylko chronić serwer, nie jest walidacją.
http://sunnyis.me/blog/secure-passwords/

strlen($password) >= 20 <- sprawdza czy hash’owanie przebiegło bez problemu. Nie powiem Ci jaki problem mógł wystąpić podczas hash’owania. Nigdy nic nie wiadomo. Hash nigdy nie będzie krótszy od 20 znaków. Linijka wyłapująca ewentualny błąd przy hash’owaniu, nie jest walidacją.
Cytat
This uses the fact that the shortest valid password hash encoding string that phpass can currently return is 20 characters long (this is the case for CRYPT_EXT_DES, whereas other hash types use even longer encoding strings). fail() is a custom function that we'll use in our sample program.

http://www.openwall.com/articles/PHP-Users-Passwords

Obsługa błędów:
O tym jakie hasło a być użytkownik jest informowany przy wypełnianiu formularza. Jest to też sprawdzane przy pomocy JS, jeżeli coś nie przejdzie walidacji, formularz nie jest nawet wysyłany.
(Wiem ze walidacja po stronie użytkownika jest niczym .Bo każdy w temacie ominie formularz i wyśle bezpośrednio do skryptu PHP. Walidacja w JS jest dla użytkowników którzy nie mają złych intencji, chcą się tylko zalogować. Formularz który na bieżąco sprawdza poprawność danych jest przyjemny dla odwiedzających(tych dobrych) ).
Dla informacji jedyne czego wymagam od użytkowników to: długość hasła nie może być mniejsza od 7 znaków. Jest to jedyna restrykcja. Nie widzę możliwości aby jakiś ciąg znaków mógł zaszkodzić mojemu skryptowi (np: ‘\złośliwy kod , %00złośliwy kod ) wszystko podejrzane zostanie haszowane i będzie hasłem.

Kończąc: jak ktoś nie zauważył już mamy błąd:D Nigdzie nie sprawdzam czy email rzeczywiście jest dłuższy od 7 znaków .Musze dodać warunek : strlen($password) >= 7 (nie może być mniejsza ! może być równa).
timon27 dzieki za odpowiedz.

---------------------------------------------------------------------------
tylko po co mi tutaj isset ?
jak zmienne $_POST będą puste zwrócą NULL. Jak wskazuje manual PHP empty(NULL) -> true, więc wstawianie tutaj warunku z isset jest bezcelowe. Z manuala:
Cytat
That means empty() is essentially the concise equivalent to !isset($var) || $var == false.

Cytat
The following things are considered to be empty:

NULL


Nie wiedziałem. Wyrażenie regularne skopiowałem z jakiejś strony która zawierała listę innych przydatnych regexp. Po przeanalizowaniu wydała mi się dobra. Znalazłem ciekawy artykuł na temat walidacji :
http://www.linuxjournal.com/article/9585
Przypadek 2 jest podobny do mojego wyrażenia podejrzewam że wspólne źródło. Nie chce iść na tak daleki kompromis jak w artykule dlatego zmieniam na:
'/^[0-9a-z _.-]+@[0-9a-z]+\.[a-z]{2,3}$/' <- teraz powinno być idealnie.

!*! dzięki za odpowiedz
reptile_rex
Taka mała ciekawostka o e-mailach, akurat ostatnio podczas testów jednostkowych na to trafiłem.

Cytat
RFC 2822:

...the local-part of the e-mail may use any of these ASCII characters:

* Uppercase and lowercase letters
* The digits 0 through 9
* The characters,! # $ % & ' * + - / =? ^ _ ` { Ś } ~
* The character "." provided that it is not the first or last character in the local-part.

Kolega wspominał o issetach ponieważ PHP rzuci notice, jeżeli odwołamy się do nieistniejącego indeksu lub zmiennej

Co do ograniczania ilości znaków hasła nie widzę zbytniego sensu.
Nie sądzę, aby wyliczenie hasha było jakoś tragiczne w skutkach dla serwera, gdybyśmy mieli hashować dłuższe ciągi znaków, jak dla mnie to już lekka paranoja.
Poniższy przykład wykona się w mgnieniu oka.

  1. for ($i = 0; $i < 1000; $i++) {
  2. $random_num = mt_rand(1, 9);
  3. $string = str_repeat($random_num, 80);
  4. echo md5($string)."\n";
  5. }
!*!
empty ma sprawdzić czy zmienna ma jakąś wartość a isset czy zmienna istnieje.
Wyrażenie nadal jest błędne np. taki email nie przejdzie
Cytat
example@example.info
example@example_xxx.com


Dodatkowo w PHP są odpowiednie filtry do ich walidacji.
Spawnm
Po co kombinacje z regexami?
filter_var( $val, FILTER_VALIDATE_EMAIL, $options );
reptile_rex
Nie wiem jak jest aktualnie, ale wiem że we wcześniejszych wersjach funkcja filter_var była zbuggowana.
Zwracała "false-positive" i tym podobne, nigdy nie zachęcałem do jej używania i sam jej nie używam.
tadeurz
Tak macie racje wyrzuca notice:
Notice: Undefined index: email in /virtual/m/y /check.php on line 8
  1. $email = (isset($_POST['email'])) ? $_POST['email']: NULL;

To oczywiście dla każdego pola.Dzięki.
--------------------------------------------
reptile_rex masz racje z tymi hasłami.Tylko ja wygenerowałem 8000 cyfrowe hasło i zhashowałem.
  1. $time = time();
  2. echo strlen($password).PHP_EOL.$time.PHP_EOL;
  3. require_once('phpClass/phpass.php');
  4. $hasher = new PasswordHash(8, TRUE);
  5. $password = $hasher->HashPassword($string);
  6. $passwordOK = ( strlen($password) >= 20 ) ? true : false ;
  7. echo $password.PHP_EOL.(time()-$time);
  8. //80000
  9. //1359799845
  10. //$P$BOGsZrlC8N7Gof1e2dY8APuPv2oTgc/
  11. //0

---------------------------------
Co do email z którymi jest największy problem( link numer 2), dla każdego prawidłolwy email to coś innego i możemy nie dojść do porozumienia:
filter_var -< ta funkcja jest bardzo tolerancyjna przepuści np: (są to prawidłowe emaile):
Cytat
first.last@[IPv6:1111:2222:3333:4444:5555:6666:7777:8888]
customer/department=shipping@iana.org
+1~1+@iana.org
{_test_}@iana.org

http://fightingforalostcause.net/misc/2006...email-regex.php
There's no perfect regular expression to validate e-mail addresses
Zakładam że jak ktoś używa takiego email?a to doskonale wie, że większość stron go odrzuci i nie będzie robił z tego problemu. Dlatego zostanę przy preg_match z uwagami !*! i wracam do A-Z:
Cytat
'/^[0-9A-Za-z _.-]+@[0-9A-Za-z_]+\.[a-z]{2,4}$/'
!*!
Cytat(Spawnm @ 1.02.2013, 22:19:32 ) *
Po co kombinacje z regexami?
filter_var( $val, FILTER_VALIDATE_EMAIL, $options );


Ponieważ to tylko dodatek, coś jak walidacja po stronie JS. Istnieje niepisana zasada że email składa się z małych liter, liczb, kropki, myślnika i podkreślnika, tak jak ma to miejsce w przypadku domen i subdomen. I to się sprawdza, zawsze i wszędzie, bo każdy tak ma.

Były swego czasu szumy że to źle i w ogóle, bo przecież można rejestrować domenę np. z polskimi znakami paweł.com i że kiedyś będą cokolwiek.google lub foo.microsoft ale na szczęście szybko umarły, bo to się po prostu nie sprawdza i jest już zbyt duża kolizja.
Wszystkie serwisy na świecie musiałby zmienić swoje mechanizmy zarówno walidacji jak i rejestracji nowych skrzynek.

A do tego dochodziłby pewnie spory prawne czy lukasz@gmail.com i łukasz@gmail.com to osobne skrzynki, czy już podszywanie się.
reptile_rex
Wszystko zależy jakiego używamy algorytmu hashowania.
Jak widzę używasz jakiejś zewnętrznej klasy, która może być mało wydajna.

Wiadomo że natywne funkcje md5 / sha1 <-- (nie polecam ich już używać do haseł) będą szybsze od tego i bardziej wydajne.

Polecam używanie hash('sha256', $string); lub hash('sha512', $string); + Sól

Zarówno sha512 jak i sha256 należą do SHA2, którego aktualnie powinno się używać.
Ze względu na długość otrzymanego hasha zmniejsza się możliwość kolizji

Jest także SHA3 (2012 rok), ale PHP chyba tego nie zaimplementował.

Pozdrawiam
tadeurz
ten skrypt nie jest jeszcze idealny.
znalazłem informacje na temat luki dotyczącej preg_match i znaku nowej nowe linii:
http://blog.php-security.org/archives/76-H...ch-filters.html

Jakby wynikało z kodu( z postu 1) mój skrypt jest na ten atak podatny, ale w praktyce tak nie jest.
Próbowałem dla :
'tadeurz@ble.pl \n ania@ble.pl' nie przeszło.
'tadeurz@ble.pl%0A ania@ble.pl' nie przeszło.
Potem jak drukowałem zmienną email to '\n' '%0a' drukował mi jak zwykłe znaki.
Wyjątek stanowił przypadek kiedy przesłałem zmienną z ‘%0A’ przy pomocy tablicy GET -> wtedy potraktował mi ten symbol jako znak nowej linii, ale mimo tego nie przeszedł walidacji preg_match.

Dlaczego ? Dodam że próbowałem dla wyrażenia regularnego zawartego w ' i " , bo znam ich różnice w przypadku echo’’ a echo””.
timon27
Cytat(tadeurz @ 1.02.2013, 21:52:39 ) *
Nikt przy zdrowych zmysłach dłuższych hasłem nie stosuje, no chyba że ma złe intencje.

Twierdzisz że nie jestem człowiekiem o zdrowych zmysłach?
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.