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 

[l0ud]

W pierwszym zapytaniu ' znajduje się w htmlspecialchars(). To błąd, który pokazuje przy okazji, że to zabezpieczenie w obecnej formie jest do... niczego (IMG:style_emoticons/default/winksmiley.jpg)

Bo skoro zapytanie działa mimo tego, oznacza to, że pojedyncze cudzysłowy ' nie są zamieniane przez htmlspecialchars(). A skoro nie są no to mamy dziurę (IMG:style_emoticons/default/winksmiley.jpg)

Rady:
- nie polecam używania htmlspecialchars() przed wrzucaniem czegokolwiek do bazy. Funkcję tą stosuj najlepiej tuż przed wyświetleniem czegoś wprowadzonego przez użytkownika (najlepiej bezpośrednio w echo '') - aby zapobiec XSS.
- zamiast htmlspecialchars() do zapytań w bazie użyj mysql_real_escape_string aby zabezpieczyć się przed SQL Injection
- skoro ładnie łączysz ciągi za pomocą . (zamiast wstawiać zmienne bezpośrednio w nich - pomiędzy " ") zacznij używać pojedynczych cudzysłowów ' ' zamiast podwójnych " ". W zapytaniach SQL natomiast ciągi opakuj " " zamiast w ' '. Słowem - zamień miejscami ' i " w zapytaniach.


Powinno wyglądać to np. tak:

[PHP] pobierz, plaintext 
#
if(mysql_num_rows(mysql_query('select * from users where user_login="'.mysql_real_escape_string($login).'"'))) $error .= '- użytkownik o podanym loginie istnieje, wybierz inny login';
[PHP] pobierz, plaintext 

A... i md5() jest już do niczego (IMG:style_emoticons/default/winksmiley.jpg) . Zamiast tego użyj przynajmniej sha1. Można jeszcze dodać tzw. sól do hasła - poszukaj.

Ten post edytował l0ud 24.06.2010, 21:51:15