Czy takie zabezpieczenia są wystarczające ? Opcje

[sheriff]

Jako iż to mój pierwszy post, witam wszystkich serdecznie (IMG:style_emoticons/default/smile.gif)

Napisałem prosty skrypt do rejestracji użytkownika. Chcę się dowiedzieć jakie błędy popełniłem i czy takie zabezpieczenia są wystarczające.
Z góry dziękuję (IMG:style_emoticons/default/winksmiley.jpg)

[PHP] pobierz, plaintext 
<?php
$error = ' ';
$fail = 0;
 
if (!isset($_POST['login'])){
 echo '
 <form method="post" action="register.php">
   <input type="text" name="login" /><br><br>
   <input type="password" name="password"/><br>
   <input type="password" name="rpassword"/><br><br>
   <input type="submit" value="Gotowe"/>
  </form>
 ';
$fail = 1; 
}
else{
$login = $_POST['login'];
$pass = $_POST['password'];
$rpass = $_POST['rpassword'];
$passmd5 = md5($pass);
mysql_connect('localhost', 'root', 'krasnal') or die('MySql fatal error.');
mysql_select_db('sheriffengine') or die('MySql fatal error.');
 
  if (strlen($login) < 4)$error .= '<br>- login musi składać się z conajmniej 5 znaków';
 
  if (strlen($pass) < 5)$error .= '<br>- hasło musi składać się z conajmniej 6 znaków';     
 
  if ($pass !== $rpass)$error .= '<br>- podane hasła muszą być takie same';
 
 
  if (preg_match("/[^A-z0-9_-]/", $login)==1)$error .= '<br>- login  może składać się wyłącznie z liter i cyfr';
 
 
  if (preg_match("/[^A-z0-9_-]/", $pass)==1 || preg_match("/[^A-z0-9_-]/", $rpass)==1 )$error .= '<br>- hasło  może składać się wyłącznie z liter i cyfr';
 
  if(mysql_num_rows(mysql_query("select * from users where user_login='".htmlspecialchars($login."'")))) $error .= '- użytkownik o podanym loginie istnieje, wybierz inny login';
 
 if ($error !== ' '){
   echo '<br>Rejestracja się nie powiodła. Zastosuj się do poniższych wskazówek:';
   $fail = 1;
    }
 
echo $error;
 
}
 
if ($fail == 0){
	mysql_query("insert into users values(NULL, '".htmlspecialchars($login)."', '".$passmd5."')"); 
  echo 'Witaj '.$login.'! Rejestracja przebiegła pomyślnie.';
}
 
?>
 
[PHP] pobierz, plaintext 

[haahh]

1. Ładniej będzie sprawdzać błędy tak: (wtedy niepotrzebna jest zmienna $fail)

[PHP] pobierz, plaintext 
if(empty($error)){
 
//dodajemy do bazy danych
 
}
else{
echo '<br>Rejestracja się nie powiodła. Zastosuj się do poniższych wskazówek:';
echo $error;
}
[PHP] pobierz, plaintext 

2. W preg_match możesz już sprawdzać, czy ma dobrą długość np. tak (ale to jak chcesz):

[PHP] pobierz, plaintext 
preg_match('/^[A-Z \'.-]{2,20}$/i', $login) // od 2 do 20 znaków
[PHP] pobierz, plaintext 

3. Wystarczy:

[PHP] pobierz, plaintext 
if(preg_match("/[^A-z0-9_-]/", $login))
 
zamiast 
 
if(preg_match("/[^A-z0-9_-]/", $login)==1)
[PHP] pobierz, plaintext 

4. Osobiście wszystkie dane od użytkownika najpierw trimuje, później mysql_real_escape_string i ewentualnie strip_tags (ale tutaj masz wyrażenia regularne).

[PHP] pobierz, plaintext 
$trimmed = array_map('trim', $_POST); // jest łatwiej bo zamiast trim($_POST['first_name']) piszesz:
$trimmed['first_name'];
[PHP] pobierz, plaintext 

5. Hasło najlepiej zabezpieczyć jakąś solą/ziarnem/czy jak to inaczej nazywają indywidualnym dla każdego usera (sporo o tym na forum)
6. Ja bym dał formularz na dół oddzielony od kodu php, tak by się wyświetlał jeżeli będą jakieś błędy. Kiedy wszystko jest ok używać exit();, ale to znowu zależy od gustu.