Forum PHP.pl > [php]logowanie - czy ten skrypt jest poprawny?

Pomoc - Szukaj - Użytkownicy - Kalendarz

Pełna wersja: [php]logowanie - czy ten skrypt jest poprawny?

No_Name

28.08.2008, 18:46:36

Stworzyłem skrypt logowania składający się z 4 stron.
Oto one:
index.php

[HTML] pobierz, plaintext 
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2//EN">
<HTML>
<HEAD>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-2">
<TITLE>Formularz logowania</TITLE>
</HEAD>
<BODY>
<form action="login.php" method="get">
  Login: <input type="text" name="login" /><br>
  Hasło: <input type="password" name="haslo" /><br>
  <input type="submit" value="Loguj" />
</form>Tutaj wpisz treść dokumentu
</BODY>
</HTML>
[HTML] pobierz, plaintext

login.php

[HTML] pobierz, plaintext 
<?php session_start();
$pol = mysql_connect("localhost","root","krasnal");
  if (!$pol) {
    echo "nie można się połączyć z bazą danych!";
    exit;
  }
$wyb = mysql_select_db("mojabaza");
  if (!$wyb) {
    echo "nie można wybrać bazy danych!";
    exit;
  };
$zap = "SELECT * FROM `uzytkownicy` WHERE login='$login' AND haslo='$haslo'";
$wyn = mysql_query($zap);
print mysql_error();
$ile = mysql_num_rows($wyn);
  if ($ile=="1") {
    $tekst = "Logowanie powiodło się!";
    $przenies = "<meta http-equiv=\"Refresh\" content=\"1; strona.php\">";
    $_SESSION['login'] = $login;
    $_SESSION['haslo'] = $haslo;
  }else{
    $tekst = "Logowanie nie powiodło się!";
    $przenies = "<meta http-equiv=\"Refresh\" content=\"1; index.php\">";
  };
@mysql_close($pol);
?>
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2//EN">
<HTML>
<HEAD>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-2">
  <?php
    echo $przenies;
  ?>
<TITLE></TITLE>
</HEAD>
<BODY>
  <?php
    echo $tekst;
  ?>
</BODY>
</HTML>
[HTML] pobierz, plaintext

strona.php

[HTML] pobierz, plaintext 
<?php session_start();
$login = $_SESSION['login'];
$haslo = $_SESSION['haslo'];
  if (!$login AND !$haslo) {
    exit;
  }
?>
 
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2//EN">
<HTML>
<HEAD>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-2">
<TITLE>Jakiś tytuł</TITLE>
</HEAD>
<BODY>
<a href="logout.php">Wyloguj się</a><br>
jakaś treść jakaś treść jakaś treść jakaś treść
jakaś treść jakaś treść jakaś treść jakaś treść
jakaś treść jakaś treść jakaś treść jakaś treść
</BODY>
</HTML>
[HTML] pobierz, plaintext

logout.php

[HTML] pobierz, plaintext 
<?php session_start();
session_destroy();
$_SESSION = array();
?>
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2//EN">
<HTML>
<HEAD>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-2">
<TITLE></TITLE>
</HEAD>
<BODY>
Zostałeś pomyślnie wylogowany<br>
<a href="index.php">Strona logowania</a>
</BODY>
</HTML>
[HTML] pobierz, plaintext

Chciałem zapytać Was czy powyższy skrypt jest bezpieczny i, jeżeli nie, jak go naprawić? Za wszystkie odpowiedzi z góry dziękuje!

Bajki

28.08.2008, 19:01:01

ja bym wysyłał formularz metodą POST

a ten fragment :

[PHP] pobierz, plaintext 
<?php
$zap = "SELECT * FROM `uzytkownicy` WHERE login='$login' AND haslo='$haslo'";
$wyn = mysql_query($zap);
print mysql_error();
$ile = mysql_num_rows($wyn);
if ($ile=="1") {
$tekst = "Logowanie powiodło się!";
$przenies = "<meta http-equiv=\"Refresh\" content=\"1; strona.php\">";
$_SESSION['login'] = $login;
$_SESSION['haslo'] = $haslo;
}else{
$tekst = "Logowanie nie powiodło się!";
$przenies = "<meta http-equiv=\"Refresh\" content=\"1; index.php\">";
};
@mysql_close($pol);
?>
[PHP] pobierz, plaintext

obudował bym w

[PHP] pobierz, plaintext 
<?php
if (!empty($_POST['login']) && !empty($_POST['haslo'])) {
(...)
}
?>
[PHP] pobierz, plaintext

mało tego ! jeszcze wcześniej sprawdził bym login i hasło przy pomocy funkcji
htmlspecialchars()" title="Zobacz w manualu PHP" target="_manual i strip_tags()" title="Zobacz w manualu PHP" target="_manual

nexis

28.08.2008, 19:07:32

Cytat(No_Name @ 28.08.2008, 19:46:36 )

[PHP] pobierz, plaintext 
<?php
...
$przenies = "<meta http-equiv=\"Refresh\" content=\"1; index.php\">";
...
?>
[PHP] pobierz, plaintext

Może jednak header" title="Zobacz w manualu PHP" target="_manual?

[PHP] pobierz, plaintext 
<?php
header('location:index.php');
exit;
?>
[PHP] pobierz, plaintext

Ponadto:

[PHP] pobierz, plaintext 
<?php
// Hasło w bazie danych radzę trzymać zakodowane - np. poprzez md5
$query = sprintf
   (
	  "SELECT COUNT(*) FROM `uzytkownicy` WHERE `login`='%s' AND `haslo`='%s'",
	  mysql_real_escape_string($_POST['login']),
	  md5($_POST['haslo'])
   );
$query = mysql_query($query) or exit(mysql_error());
$count = mysql_fetch_row($query);
if (1 != $count[0]) {
   exit('Komunikat błędu!');
}
$_SESSION['login'] = $_POST['login'];
$_SESSION['haslo'] = md5($_POST['haslo']);
?>
[PHP] pobierz, plaintext

pyro

28.08.2008, 19:08:48

skrypt nie jest ani poprawny (chyba, że masz włączone register_globals), ani bezpieczny (nie filtrujesz danych w zaden sposob)

No_Name

28.08.2008, 19:11:37

co sugerujesz zmienić?

Bajki

28.08.2008, 19:13:16

podepne sie : wszystkie $login i $haslo musisz zamienic na $_POST['login'] $_POST['haslo'] albo $_GET[] jak tam wolisz ;p

pyro

28.08.2008, 19:16:57

nic zmienic, a filtrowac dane, i pobierac dane z $_POST, jesli masz wylaczone register_globals

No_Name

28.08.2008, 19:29:39

Teraz pliki po zmianie wyglądają tak:

index.php

[HTML] pobierz, plaintext 
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2//EN">
<HTML>
<HEAD>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-2">
<TITLE>Formularz logoawnia</TITLE>
</HEAD>
<BODY>
<form action="login.php" method="post">
  Login: <input type="text" name="login" /><br>
  Hasło: <input type="password" name="haslo" /><br>
  <input type="submit" value="Loguj" />
</form>Tutaj wpisz treść dokumentu
</BODY>
</HTML>
[HTML] pobierz, plaintext

login.php

[HTML] pobierz, plaintext 
<?php session_start();
$login = strip_tags($_POST['$login']);
$haslo = strip_tags($_POST['$haslo']);
$login = trim($login);
$haslo = trim($haslo);
$pol = mysql_connect("localhost","root","krasnal");
  if (!$pol) {
    echo "nie można się połączyć z bazą danych!";
    exit;
  }
$wyb = mysql_select_db("mojabaza");
  if (!$wyb) {
    echo "nie można wybrać bazy danych!";
    exit;
  };
$zap = "SELECT * FROM `uzytkownicy` WHERE login='$login' AND haslo='$haslo'";
$wyn = mysql_query($zap);
print mysql_error();
$ile = mysql_num_rows($wyn);
  if ($ile=="1") {
    $tekst = "Logowanie powiodło się!";
    $przenies = "<meta http-equiv=\"Refresh\" content=\"1; strona.php\">";
    $_SESSION['login'] = $login;
    $_SESSION['haslo'] = $haslo;
  }else{
    $tekst = "Logowanie nie powiodło się!";
    $przenies = "<meta http-equiv=\"Refresh\" content=\"1; index.php\">";
  };
@mysql_close($pol);
?>
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2//EN">
<HTML>
<HEAD>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-2">
  <?php
    echo $przenies;
  ?>
<TITLE></TITLE>
</HEAD>
<BODY>
  <?php
    echo $tekst;
  ?>
</BODY>
</HTML>
[HTML] pobierz, plaintext

strona.php

[HTML] pobierz, plaintext 
<?php session_start();
$login = $_SESSION['login'];
$haslo = $_SESSION['haslo'];
  if (empty($_POST['login']) && empty($_POST['haslo'])) {
    exit;
  }
?>
 
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2//EN">
<HTML>
<HEAD>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-2">
<TITLE>Jakiś tytuł</TITLE>
</HEAD>
<BODY>
<a href="logout.php">Wyloguj się</a><br>
jakaś treść jakaś treść jakaś treść jakaś treść
jakaś treść jakaś treść jakaś treść jakaś treść
jakaś treść jakaś treść jakaś treść jakaś treść
</BODY>
</HTML>
[HTML] pobierz, plaintext

Jeżeli umieszczę takie pliki na serwerze to będzie wszystko ok?

pyro

28.08.2008, 19:37:46

nie.

[PHP] pobierz, plaintext 
<?php
$zap = "SELECT * FROM `uzytkownicy` WHERE login='$login' AND haslo='$haslo'";
?>
[PHP] pobierz, plaintext

admin'/*

// ADD

użyj mysql_real_escape_string() lub addslashes()

No_Name

29.08.2008, 08:42:14

po zmianie plik login.php wygląda tak:

[HTML] pobierz, plaintext 
<?php session_start();
$login = strip_tags($_POST['$login']);
$haslo = strip_tags($_POST['$haslo']);
$login = trim($login);
$haslo = trim($haslo);
$login = addslashes($login);
$haslo = addslashes($haslo);
$pol = mysql_connect("localhost","root","krasnal");
  if (!$pol) {
    echo "nie można się połączyć z bazą danych!";
    exit;
  }
$wyb = mysql_select_db("mojabaza");
  if (!$wyb) {
    echo "nie można wybrać bazy danych!";
    exit;
  };
$zap = "SELECT * FROM `uzytkownicy` WHERE login='$login' AND haslo='$haslo'";
$wyn = mysql_query($zap);
print mysql_error();
$ile = mysql_num_rows($wyn);
  if ($ile=="1") {
    $tekst = "Logowanie powiodło się!";
    $przenies = "<meta http-equiv=\"Refresh\" content=\"1; strona.php\">";
    $_SESSION['login'] = $login;
    $_SESSION['haslo'] = $haslo;
  }else{
    $tekst = "Logowanie nie powiodło się!";
    $przenies = "<meta http-equiv=\"Refresh\" content=\"1; index.php\">";
  };
@mysql_close($pol);
?>
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 3.2//EN">
<HTML>
<HEAD>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-2">
  <?php
    echo $przenies;
  ?>
<TITLE></TITLE>
</HEAD>
<BODY>
  <?php
    echo $tekst;
  ?>
</BODY>
</HTML>
[HTML] pobierz, plaintext

Mogę już te pliki umieścić na serwerze?

pyro

29.08.2008, 09:20:49

można by jeszcze zamiast tych tagów meta dawać header('Location: strona'); ale to da sie wyżyć, przed mysql_close nie musisz dawać znaku małpy, a poza tym już jest chyba ok

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.