Ankündigung

Einklappen
Keine Ankündigung bisher.

Sicherheitsprobleme bei Login-Formular

Einklappen

Neue Werbung 2019

Einklappen
X
  • Filter
  • Zeit
  • Anzeigen
Alles löschen
neue Beiträge

  • Sicherheitsprobleme bei Login-Formular

    Hallo liebe Community,

    ich habe gerade ein wenig Probleme mit Mazedonischen hackern und nach mehr als 5 Stunden Logfile analysieren komme ich nur auf mein Loginskript zurück, das anscheinend unsicher ist.

    Was mach ich falsch?

    Code:
    if(isset($_POST['login_button']) && isset($_POST['username']))
    {
     $db=mysql_query("SELECT passwort,username,id FROM user WHERE username=".$_POST['username']." LIMIT 1")or die(mysql_error());
     while($db_array=mysql_fetch_array($db))
     {
       if(md5($_POST['passwort'])==$db_array['passwort']) $_SESSION['is_drin'] = "true";
     }
    }

    OK, ich sollte dringend die übergebenen POST Variablen per mysql_real_escape_string() umwandeln, aber auch so konnte doch nicht wirklich was schief gehen ... oder?


    Vielen Dank für Eure Hilfe,

    hamlet


  • #2
    Also ich prüfe generell alle Daten die auf die DB loslasse. Generell jede Art von Code filtern / umwandeln, bestimmten Typen wie Benutzername und/oder Passwort auf Zeichen und Länge.

    Zusätzlich logge ich über meine DB Klasse alle An/Abfragen in LogFile. In meiner User Klasse habe ich ein zusätzliches Handling für fehlerhaften Login drin, zum einen werden die Daten die gesendet wurden in einem Logfile gesichert und zum andern geht geht eine Warn Email bei mehr als 3 falschen Versuchen an mich raus.

    Bin mal gehackt worden und seit dem ein bisschen paranoid
    Gruß Werner
    Mein kleines Projekt: Fussball Satrup
    Wird ein OpenSource CMS für Fussballvereine

    Kommentar


    • #3
      Wie du schon erkannt hast ist bei dir SQL Injection möglich. Daher kann da jede Menge passieren! Du solltest unbedingt auch deine anderen Skripte überprüfen ob dort ebenfalls SQL Injection möglich ist!

      @woskamp: Du bist nicht paranoid. Daten müssen IMMER so geprüft werden wie du es beschrieben hast! Jegliche Daten die von außerhalb kommen, also in GET, POST und so weiter stehen müssen auf jeglichen Inhalt überprüft werden.

      Kommentar


      • #4
        PHP-Code:
        "... WHERE username=".$_POST['username']." LIMIT..." 
        Da würde mysql_real_escape_string() auch nix mehr bringen. Denn in dem Fall muss man keine Hochkommata einschleusen, da reicht auch ein Leerzeichen für die SQL-Injection. Sollte eher so aussehn:
        PHP-Code:
        "... WHERE username='".mysql_real_escape_string($_POST['username'])."' LIMIT..." 
        Ich frag mich übrigens grad wie du den User identifizieren willst.

        Kommentar


        • #5
          PHP-Code:
          $_SESSION['is_drin'] = "true"
          lol ich kipp gleich um... so stell ich mir sex mit einer Informatik studentin vor.
          und nun zurück zum problem:
          PHP-Code:
          if(isset($_POST['login_button']))
          {
          $user mysql_real_escape_string($_POST['username']);
          $pw md5($_POST['passwort']);
          if(empty(
          $user) || empty($pw)){
          echo 
          "Eingabefelder sind leer";
          }else{
          $sql sprintf("SELECT id FROM user WHERE username = '%s' AND passwort = '%s' ",$user,$pw);
          $sql mysql_query($sql) or die(mysql_error());
          $row mysql_fetch_object($sql);
          if(empty(
          $row->id)){
          echo 
          "Benutzer exestiert nicht";
          }else{
          $_SESSION['ist_drin'] = true;
          echo 
          "Jaaa du bist drin!";
          }

          so würde es besser funktionieren, wenn du dir gleich die ID des benutzers holst wo der name und das passwort übereinstimmt anstatt erst den benutzer prüfen und dann das passwort

          Zitat von gs93 Beitrag anzeigen
          Ich frag mich übrigens grad wie du den User identifizieren willst.
          ganz einfach $user = rand(0,100);
          apt-get install npm -> npm install -g bower -> bower install <package> YOLO https://www.paypal.me/BlackScorp | Mein Youtube PHP Kanal: https://www.youtube.com/c/VitalijMik

          Kommentar


          • #6
            Hey, vielen Dank für die schnellen Rückmeldungen.

            Ich werd eure Tipps so nach und nach versuchen umzusetzen.


            Mein Skript oben war nicht ganz komplett. Die UserID geht natürlich auch in die Session. Ich wollt nur den Quellcode nicht ganz so verkomplizieren.
            Die ' waren wohl (auch) ein Grund für Problems (sind zwar von nem Select gekommen, aber ... wie ihr gesagt habt .... vertrau keinem POST)

            *gruml*


            Danke

            *jetzt fleißig code 'versichert'*

            Kommentar


            • #7
              Zitat von hamlet.ost Beitrag anzeigen
              Die ' waren wohl (auch) ein Grund für Problems (sind zwar von nem Select gekommen, aber ... wie ihr gesagt habt .... vertrau keinem POST)
              Vertrau auf gar nichts.. Ob die daten nun via Post, get oder sonst wie kommen spielt absolut keine rolle.

              Wichtig ist, das sie:

              1. geprüft werden.. oder überschrieben, oder typumwandlung
              2. siehe punkt 1
              3. siehe punkt 1
              4. bei SQL alles in Hochkommas + mysqlrealescape sei es auch nur eine zahl z.b. die "ID"
              5. Alle Angriffspunkte müssen dir klar sein. und das ist jedes mal dort wo etwas von extern ins skript kommt oder durch skript durchläuft (variablen, SQL, Dateisystem usw.)


              ich würde es so machen:
              PHP-Code:
              //im login-modus.

              $sql "SELECT passwort,username,id FROM user WHERE username='".mj($_POST['username']).'"  AND passwort='".mj($_POST['passwort'])."'  LIMIT 1"

              while($db_array  = @mysql_fetch_array(  @mysql_query($sql) )) 
              {
                $_SESSION['
              is_drin'] = "true";  //bei Informatik-Studentin :-)
                echo '
              drin'; exit;  // oder return true;  wenn alles als funtktion
              }

              //Falscher Login
              echo '
              Du bist leider nicht weiter'; 

              Erklärung:
              Benutzer + pw gleich in einem statement prüfen.
              stimmt, der query nicht, gibts nichts zu fechten und somit keine while.
              so erübrigen sich auch alle fälle wo ein Wert fehlt. du kannst auch via IF-Abfrage.
              gibt also nur richtig oder falsch...
              $db_array brauchst du nicht wirklich hier.


              Und Dann dies Funktion von mir nutzen siehe unten. Warum über diese funktion?
              1. zum escapen
              2. sql-statement bleibt leserlich
              3. erweiterbar für die zukunft, z.b. wenn du noch mehr sicherheit einbauen willst. z.b. bekannte injections rausparsen oder ähnliches


              PHP-Code:
              // My-SQL -> Anti - Injection funktion
              function mj($tmp) { return mysql_real_escape_string($tmp); } 
              www.scriptforums.com - Foren für Skripts
              www.ragonvote.net - Kostenlose Umfragen
              www.ragonsoft.com - PHP und Android Apps (z. B. Knoten Video Guide)

              Kommentar


              • #8
                bei SQL alles in Hochkommas + mysqlrealescape sei es auch nur eine zahl z.b. die "ID"
                Zahlen/Integer werden nicht von Hochkommas eingeschlossen und sollten gecastet werden. Sei es durch
                Code:
                $int = (int)$int;
                oder z.B. mit Hilfe der Funktion PHP: intval - Manual.

                Zudem macht es keinen guten Eindruck, wenn du hier mit falschem Code um dich wirfst. Den Fehlerkontrolloperator (@) solltest du auch aus deinem Code entfernen.

                Code:
                function mj($tmp) { return mysql_real_escape_string($tmp); }
                ist ebenfalls recht sinnfrei.
                http://hallophp.de

                Kommentar


                • #9
                  Zitat von Asipak Beitrag anzeigen
                  PHP-Code:
                  $int = (int)$int
                  korrekt... die typumwandlung bzw. "sicherstellen". ein hochkomma zuviel schadet aber nicht.. eines zu wenig schon.

                  Zudem macht es keinen guten Eindruck, wenn du hier mit falschem Code um dich wirfst. Den Fehlerkontrolloperator (@) solltest du auch aus deinem Code entfernen.
                  Es gehört natürlich zum obigen code dazu. alleine sonst ist es schon sinnfrei.
                  wieso den Fehleroperator raus? Ein leerer fetch erzeugt ein fehler. darum hab ich den drinn (code sparen).
                  normalerweise ist er natürlich nicht drin.... damit alle felher schön angezeigt werden.
                  www.scriptforums.com - Foren für Skripts
                  www.ragonvote.net - Kostenlose Umfragen
                  www.ragonsoft.com - PHP und Android Apps (z. B. Knoten Video Guide)

                  Kommentar


                  • #10
                    Ein leerer fetch erzeugt ein fehler.
                    Nicht, wenn mysql_query() eine gültige Resource-ID zurückliefert. Und das tut die Funktion auch, wenn keine Übereinstimmungen in der Datenbank gefunden wurden.
                    http://hallophp.de

                    Kommentar


                    • #11
                      @advanced_phpler: Du hast einen Fehler bei den einfachen und doppelten Hochkommas, deswegen läuft das mit dem highlighten auch schief.
                      @ zu benutzen ist trotzdem nicht sauber und erschwert die Fehlersuche. Außerdem, einer Variable den String "true" zuzuweisen...
                      MfG

                      Kommentar


                      • #12
                        Zitat von gs93 Beitrag anzeigen
                        @advanced_phpler: Du hast einen Fehler bei den einfachen und doppelten Hochkommas, deswegen läuft das mit dem highlighten auch schief.
                        @ zu benutzen ist trotzdem nicht sauber und erschwert die Fehlersuche. Außerdem, einer Variable den String "true" zuzuweisen...
                        MfG
                        naja Ansichtssache mit dem @ gerade beim Login mach ich das öfters damit - sollte Errorreporting off mal vergessen gehen usw. ja besser: statt "true" würd ich's in eine funktion oder include setzen und einfach true od false zurückgeben. Hochkomma muss ich mir nachher schnell Abschauen irgendwas muss falsch sein.
                        www.scriptforums.com - Foren für Skripts
                        www.ragonvote.net - Kostenlose Umfragen
                        www.ragonsoft.com - PHP und Android Apps (z. B. Knoten Video Guide)

                        Kommentar


                        • #13
                          error_reporting sollte immer an sein. Dagegen solltest du im Live-Betrieb display_errors auf 0 setzen und stattdessen die Fehlermeldungen mitloggen.

                          statt "true" würd ich's in eine funktion oder include setzen und einfach true od false zurückgeben.
                          Es ging wohl eher darum, dass man mit einem booleschen Wert besser arbeiten kann, als mit einem String, wenn man den Loginstatus überprüfen möchte.
                          http://hallophp.de

                          Kommentar


                          • #14
                            Zitat von advanced_phpler Beitrag anzeigen
                            Erklärung:
                            Benutzer + pw gleich in einem statement prüfen.
                            stimmt, der query nicht, gibts nichts zu fechten und somit keine while.
                            so erübrigen sich auch alle fälle wo ein Wert fehlt. du kannst auch via IF-Abfrage.
                            gibt also nur richtig oder falsch...
                            $db_array brauchst du nicht wirklich hier.
                            Diesem Hinweis (samt dem Code) sollte man aber auch auf keinen Fall folgen, weil dabei das Passwort im Klartext in der Datenbank seht und selbst beim Hashen des Passwortes und anschließender Überprüfung über ein Statement ein guter Salted Hash nicht mehr möglich ist!

                            Kommentar


                            • #15
                              Zitat von Sirke Beitrag anzeigen
                              Diesem Hinweis (samt dem Code) sollte man aber auch auf keinen Fall folgen, weil dabei das Passwort im Klartext in der Datenbank seht und selbst beim Hashen des Passwortes und anschließender Überprüfung über ein Statement ein guter Salted Hash nicht mehr möglich ist!
                              Natürlich hab ich nicht jede erdenkliche situation berücksichtig.... sicher ist es allemal. Sofern man noch den Login bzw. die fehlschläge countet.

                              klar so ist es natürlich noch sicherer:
                              WHERE passwort = md5 ( mj( $pw ) )

                              Als lösung mit verslüsselung oder anderer verschlüsselung
                              und wieso sollte das ein problem sein wenn das pw im klartext steht? Ein Angreifer hat kein zugriff auf die DB.. nur der admin.


                              und wenn der login wirklich 100% absolut sicher sein, soll dann muss man mit account-blocken oder zeitsperre arbeiten... Nach z.b 5 fehlversuchen, account blocken (24h)... da kommt kein hacker dagegen an. dem user dann eine entsperr-möglichkeit geben.

                              So arbeiten übrigens die Banken mit den Geldautomaten.. und die wissen warum. Da hilft dann auch keine dictionary-attacke mehr.


                              Bei der einweg-verschlüsselung kann man das passwort dem Kd nicht mehr "zeigen" oder zusenden.. er muss es ändern.. und jede andere verschlüsselung(streichliste aussen vor) kann gehackt werden.

                              im prinzip alles was man wieder ent-schlüsseln kann.

                              hier nochmals etwas schöner geschrieben:
                              PHP-Code:
                                  if($id == '' || $pw == '' ) { return false;}    // und wer sich das hier spraren will, einfach ein @mysql_query schreiben beim
                                  
                              while(mysql_fetch_array(mysql_query("SELECT * FROM users WHERE ID=".mj($id)." AND pw='".md5(mj($pw))."' Limit 1")))
                                  {return 
                              true;}
                                  
                              //Falsches Daten...
                                  
                              return false
                              für die welche die while nicht mögen. aber je nach server verhält es sich anders mit der resource, daher nehm ich eher die while.
                              PHP-Code:
                               //Variante ohne while, mit IF 
                                    
                              if( $id == '' || $pw == '' ) { return false;}  // wird gebraucht, sonst klappts nicht...
                                   
                              if ( mysql_query("SELECT * FROM users WHERE ID=".mj($id)." AND pw='".md5(mj($pw))."' Limit 1")     )
                                   {return 
                              true;} else {return false;} 
                              jedes "false" in diesem Code muss aber registiert werden. also in konto schreiben wie viele fehlversuche, dann massnahmen.
                              z.b. Zeitsperre. also erst 24h später wieder logins zulassen usw. im extremfall konto blocken.
                              www.scriptforums.com - Foren für Skripts
                              www.ragonvote.net - Kostenlose Umfragen
                              www.ragonsoft.com - PHP und Android Apps (z. B. Knoten Video Guide)

                              Kommentar

                              Lädt...
                              X