[skrypt] Logowanie Opcje

[sebekzosw]

Witam!

Mam taki skrypt logowania opartego o sesje - moje pytanie brzmi: Czy taki skrypt jest bezpieczny?:

[PHP] pobierz, plaintext 
<?php
    function DodawanieDoBazy($ciag) {
        $wynik = stripslashes(strip_tags(addslashes(trim($ciag))));
        $wynik = str_replace(array("\"", "'", "\\", '\"', "\'", "<", ">", "&nbsp;"), array("&quot;", "'", "\", "&quot;", "'", "&lt;", "&gt;", " "), $wynik);
        $wynik = preg_replace(array("/\=/","/\#/","/\sOR\s/"), "", $wynik);
        return $wynik;
    }
 
 
    if(isset($_POST["logowanie"]) && isset($_POST['login']) && isset($_POST['login'])) {
        $logon_failure = '';
        if(empty($_POST["login"])) { $logon_failure .= "Wpisz login<br />"; }
        if(empty($_POST["password"])) { $logon_failure .= "Wpisz hasło<br />"; }
        $login = DodawanieDoBazy($_POST["login"]);
        $password = md5($_POST["password"]);
 
        if(empty($logon_failure)) {
            if(!mysql_fetch_row(mysql_query("SELECT * FROM users WHERE login='".$login."' AND pass='".$password."'"))) {
                $logon_failure .= "Nieprawidłowy login lub hasło";
            } elseif(!mysql_fetch_row(mysql_query("SELECT * FROM users WHERE login='".$login."' AND activity='1'"))) {
                $logon_failure .= "Konto jest nieaktywne";
            }
 
            if(empty($logon_failure)) {
                $_SESSION["login"] = $login;
                 $_SESSION['ip'] = $_SERVER['REMOTE_ADDR'];
                 $_SESSION['browser'] = $_SERVER['HTTP_USER_AGENT'];
            } else {
                echo $logon_failure;
            }
        } else {
            echo $logon_failure;
        }
    }
    echo "<div id=\"menu\">";
    if(UZYTKOWNIK) {
        echo "Zalogowany jako ".$_SESSION["login"];
    } else {
 
 
?>
    <form action="" method="post" id="logowanie">
        <fieldset>
            <dl>
                <dt>Login</dt>
                 <dd><input type="text" name="login" id="login" value="Login" onblur="if(this.value=='') this.value='Login';" onfocus="if(this.value=='Login') this.value='';" /></dd>
 
                <dt>Hasło:</dt>
                 <dd><input type="password" name="password" id="password" value="Hasło" onblur="if(this.value=='') this.value='Hasło';" onfocus="if(this.value=='Hasło') this.value='';" /></dd>
            </dl>
        </fieldset>
        <div style="float:right;"><input type="submit" name="logowanie" value="Zaloguj się" onclick="WyslijFormularz('logowanie'); return false;" /></div>
    </form>
    <img src="http://test.php.pl/www/gfx/login_loading.gif" id="ladowanie" alt="" style="display:none;float:left;" />
    <div id="wynik"></div>
 
    <p><a href="/rejestracja/">Rejestracja</a>
        <br />
        <a href="/przypomnij/">Przypomnij hasło</a>
 
 
 <?
 }
 ?>
 </div>
[PHP] pobierz, plaintext 

Wszystkie sugestie na temat polepszenia mile widziane (IMG:style_emoticons/default/smile.gif)

Ten post edytował erix 19.08.2009, 23:10:18

Powód edycji: [erix] przeniosłem

[Quantum]

[PHP] pobierz, plaintext 
ie"]) && isset($_POST['login']) && isset($_POST['login'])
[PHP] pobierz, plaintext 

w jakim celu sprawdzasz 2 razy ? (IMG:style_emoticons/default/biggrin.gif) dodatkowo dziwna nazwa funkcji, DodawanieDoBazy, lepiej byłoby FiltrujZapytanie, choć i tak jestem zwolennikiem nazewnictwa ang. używaj http://www.php.net/mysql_real_escape_string.
Dlaczego raz używasz ' a raz " ? zastąp cudzysłów apostrofem, zwiększa to szybkość działania skryptu.
Co do bezpieczeństwa.. MD5 ? ta funkcja skrótu teoretycznie w nowych aplikacjach pojawiać się nie powinna, zastąp ją SHA-1. Od siebie dodam, że lepiej napisałbyś to obiektowo, proceduralne - gorzej się czyta, po 2 tygodniach przerwy nie będziesz wiedział jak on nawet działał, OOP daje większe możliwości, na ten przykład PDO - nie musiałbyś martwić się o bezpieczeństwo, zapytania są tam filtrowane. Dodatkowo rozbudowa takich skryptów jest o wiele szybsza i prostsza.

[PHP] pobierz, plaintext 
if(!mysql_fetch_row(mysql_query("SELECT * FROM users WHERE login='".$login."' AND pass='".$password."'"))) {
[PHP] pobierz, plaintext 

zastąp tym, będzie wydajniej

[SQL] pobierz, plaintext 
SELECT count(*) FROM users WHERE login='".$login."' AND pass='".$password."'
[SQL] pobierz, plaintext 

Ten post edytował sniffer32 20.08.2009, 06:14:23