Sicheres Login?

minder

Hi

Ist das ein sicheres Login?

PHP

if ($_POST['user'] != "" AND $_POST['password'] != "")
        {
            if ($this->mysql->connect())
            {
                if ($this->mysql->entry_count("user", "name='".$_POST['user']."' AND password='".md5($_POST['password'])."' AND block='0'") > 0)
                {
                    $this->result = $this->mysql->read_entry("user","name='".$_POST['user']."' AND password='".md5($_POST['password'])."'","", 2);


                    $_SESSION['access'] = $this->result[0]->access;
                    $_SESSION['name'] = $this->result[0]->name;
                    $_SESSION['firstname'] = $this->result[0]->firstname;
                    $_SESSION['lastname'] = $this->result[0]->lastname;
                    $_SESSION['email'] = $this->result[0]->email;
                    $_SESSION['state'] = 1;

Alles anzeigen

die else sachen fehlen

crAzywuLf

Zitat von minder

Hi

Ist das ein sicheres Login?

PHP

if ($_POST['user'] != "" AND $_POST['password'] != "")
        {
            if ($this->mysql->connect())
            {
                if ($this->mysql->entry_count("user", "name='".$_POST['user']."' AND password='".md5($_POST['password'])."' AND block='0'") > 0)
                {
                    $this->result = $this->mysql->read_entry("user","name='".$_POST['user']."' AND password='".md5($_POST['password'])."'","", 2);


                    $_SESSION['access'] = $this->result[0]->access;
                    $_SESSION['name'] = $this->result[0]->name;
                    $_SESSION['firstname'] = $this->result[0]->firstname;
                    $_SESSION['lastname'] = $this->result[0]->lastname;
                    $_SESSION['email'] = $this->result[0]->email;
                    $_SESSION['state'] = 1;

Alles anzeigen

die else sachen fehlen

Äh nein
..................................

Pion

Wie sieht den die Klasse aus?
Sprich mysql->entry_count

mfg

minder

public function entry_count ($table, $where)
{

$this->sql = "SELECT * FROM ".$this->prefix.$table." WHERE ".$where;

$this->query = mysql_query($this->sql) or die("Shit happens (count)");

return mysql_num_rows($this->query);
}

minder

Zitat von crAzywuLf

Äh nein
..................................

genaue beschreibung

Pion

OKay also folgendes:

Schau dir mal "Mysql Injection" an

read_entry kannst uns auch mal noch zeigen, also dein größtes Problem was das scirpt unsicher macht ist http://de.wikipedia.org/wiki/SQL-Injection

minder

Ja schon aber das geht ja nur wenn die daten per get übergeben werden?! aber ich benutze post

Pion

Was für ein Irrglaube, das geht überall da wo der Usermanipulieren kann spricht get, post, cookie etc

mfg

crAzywuLf

Zitat von minder

genaue beschreibung

Hättest du die Suche benutzt hättest du jetzt mindestens schon 10 Lösungen von Usern die diesen und letzten Monat gepostet haben...
Selbst meine Oma hätte was zu dem Thema hier im Forum gefunden.

PHP

<?php
$user = $_POST['user']; 
$password = $_POST['password'];
if (!empty($user) && !empty($password))
{
    if ($this->mysql->connect())
    {
        if ($this->mysql->entry_count("user", "`name` = '" . mysql_real_escape_string($user) . "' AND `password` = '" . md5($password) . "' AND `block` = '0'") > 0)
        {
            $this->result = $this->mysql->read_entry("user","`name` = '" . mysql_real_escape_string($user) . "' AND `password` = '" . md5($password) . "'", "", 2);


            $_SESSION['access'] = $this->result[0]->access;
            $_SESSION['name'] = $this->result[0]->name;
            $_SESSION['firstname'] = $this->result[0]->firstname;
            $_SESSION['lastname'] = $this->result[0]->lastname;
            $_SESSION['email'] = $this->result[0]->email;

            # ...

?>

Alles anzeigen

mysql_real_escape_string()
Da das Passwort mit md5 versiehst, ist das schon mal ganz ok.
Den Namen könntest du noch mit einem regulären Ausdruck oder Funktionen wie ctype_alnum() prüfen, hier nach dem wie die bei dir aussehen und welche Zeichen du zulässt\zulassen willst.

Ansonsten währe noch ein Problem, dass der Query von $this->mysql->read_entry warscheinlich irgendwie so aussieht:

Zitat

SELECT * FROM....

was nicht gut ist...
Du solltest nur Felder auslesen die du auch benutzt.

Bitteschön, aber die Suche währe einfacher gewesen.

EDIT:
p.s.
mit COUNT(*) macht die Funktion erst richtig Sinn^^

PHP

<?php
public function entry_count ($table, $where)
{
    $this->sql = "SELECT COUNT(*) FROM " . $this->prefix.$table . " WHERE " . $where;
    $this->query = mysql_query($this->sql) or die("Shit happens (count)");
    return mysql_num_rows($this->query);
} 
?>

Pion

@treadstarter
So und jetzt bin ich mal so nett und gibt dir mal nen Code der vllt interessant für dich wäre:

Code

$querystring = preg_replace('/(FROM|UPDATE|INTO|TABLE|JOIN) (.+)/Us', '$1 '.$pref.'_$2', $querystring);

Das setzz du in deine function entry_count ein, dann kannst du dir sparen immer Sachen an die Funktion vom Query übergeben zu müssen:)

@crazywolf
!empty($user) oder vllt doch einfach isset($user)

crAzywuLf

Zitat

@crazywolf
!empty($user) oder vllt doch einfach isset($user)

zwischen isset() und empty() ist ein Unterschied und für mich ist eigentlich nur interessant ob die Variable nicht leer ist und nicht ob die Variable existiert:p

PHP

$test = "";
if(isset($test))
{
    echo "Jo";
}

The User

Bei Formularen werden leere Felder mitgeschickt.

md5 ist aber auch nicht ganz das Wahre. Siehe Hashing.

minder

So müssten doch Mysql injections verhindert werden

PHP

public function read_entry($table, $where, $order, $type)
    {
        switch($type)
        {
            case 1:
            {
                $this->sql = "SELECT COUNT(*) FROM ".$this->prefix.$table;
            }break;


            case 2:
            {
                $this->sql = "SELECT COUNT(*) FROM ".$this->prefix.$table." WHERE ".mysql_real_escape_string($where);
            }break;
            case 3:
            {
                $this->sql = "SELECT COUNT(*) FROM ".$this->prefix.$table." WHERE ".mysql_real_escape_string($where)." ORDER BY ".$order;
            }break;
            case 4:
            {
                $this->sql = "SELECT COUNT(*) FROM ".$this->prefix.$table." ORDER BY ".$order;
            }break;
        } // switch


        $this->result = mysql_query($this->sql) or die('Fehler beim Lesen der Tabelle '.$table.'!');


        for ($i=0; $i < mysql_num_rows($this->result); $i++)
        {
            $this->mysqldata[$i] = mysql_fetch_object($this->result);
        }


        return $this->mysqldata;
    }

Alles anzeigen

Pion

Theoretisch schon...
ist $order eine get variable ?

Der code naja

Bandit

Diese Funktion ist völliger Quatsch! Was kommt da am Ende raus? Die Anzahl der Datenzeilen, die zutreffen, die Daten aber nicht.

Und $type sollte nicht am Ende der Parameterliste stehen, da diese Variable auf jeden Fall benötigt wird. $where und $order sollten vorinitialisiert sein.

minder

Hi

Also diese Funktion wird eigentlich von vielen anderen funktionen aufgerufen daher ist es nicht möglich die $where und $table variable am anfang zu definieren. Das mit dem switch ist auch nur weil einzelne funktionen nicht immer eine where eingrenzung brauchen

Bandit

Zitat von minder

Also diese Funktion wird eigentlich von vielen anderen funktionen aufgerufen daher ist es nicht möglich die $where und $table variable am anfang zu definieren.

Tja, klassischer Fall von falscher Planung. Pflichtparameter gehören an den Anfang, optionale Parameter dahinter.

Sicheres Login?

Tags

Benutzer online in diesem Thema