Ankündigung

Einklappen
Keine Ankündigung bisher.

[Erledigt] Kontrolle $_GET-Werte - Whitelist

Einklappen

Neue Werbung 2019

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

  • [Erledigt] Kontrolle $_GET-Werte - Whitelist

    Hallo,

    ich arbeite weiter an der Kontrolle von $_GET-Werten, die über URL-Parameter übergeben werden. Nach hilfreichen Kritiken von mermshaus (danke!) sieht es nun so aus:

    Alle Integer werden mit intval() überprüft. Müsste gegen Manipulationen sicher sein.

    Andere mögliche Werte werden mit einer Whitelist geprüft. Die erlaubten Werte werden aus der DB in ein Array eingelesen und mit dem übergebenen Wert abgeglichen. Beispiel:
    PHP-Code:
    // Anti-SQL-Injection Parameter "um" (whitelist)
    function param_um_check($um) {
    include(
    _INCPFAD."db.php");
        
    $array_umname = array();
        
    $conttab_de TAB_CONT."de";
        
    $um_name mysql_query("SELECT umname FROM ".$conttab_de." GROUP by umname;");
               while (
    $zeile mysql_fetch_object($um_name)) {
               
    $umname $zeile->umname;               
                   if (!empty(
    $umname)) {
                      
    $array_umname[] = substr (strrchr($umname"_"), 1);
                   }
               }
        if (!
    in_array($um$array_umname)) {             
        echo 
    "<p>Not allowed</p>";
        exit;
       }
    // Anti-SQL-Injection Parameter "um" 
    Anm.: Ich weiß, dass ich noch das alte mysql verwende. Das kommt später dran.

    In der index.php wird die Funktion so aufgerufen:
    PHP-Code:
    if ($_GET['um']) {
    $um strip_tags($_GET['um']);
    param_um_check($um);

    Was meint ihr - kann man den Parameter "um" noch manipulieren?
    Danke für gute Tipps.


  • #2
    Auch wenn ich mir jetzt Feinde mache, aber Du solltest Deine Selbsteinschätzung überdenken!
    Alle Integer werden mit intval() überprüft.
    intval() prüft gar nichts, es konvertiert den übergebenen Wert auf Grundlage der evtl. angegebenen Basis.
    Dazu kommt, daß Du in Deinem geposteten Code kein einziges Mal intval() verwendest!

    Weiterhin spricht
    Anm.: Ich weiß, dass ich noch das alte mysql verwende. Das kommt später dran.
    nicht für fortgeschrittene Kenntnisse...
    Competence-Center -> Enjoy the Informatrix
    PHProcks!Einsteiger freundliche TutorialsPreComposed Packages

    Kommentar


    • #3
      Ach weißt du, Arne,

      ich habe zum Hobby eine Webseite und nicht immer Zeit, daran zu arbeiten. Aber ich bemühe mich nach bestem Wissen um Sicherheit.

      Da fände ich Tipps zu meinen Scripten besser als Urteile über meine Selbsteinschätzung. Ich stufe mich ja gerne zurück, wenn das geht.

      Dass intval() nicht prüft, weiß ich. Aber es filtert doch wohl aus einem String alles, was kein Integer ist - richtig? Somit kann niemand über einen so behandelten Wert eines $_GET-Parameters Schadcode einschleusen. So meine Überlegung - und meine Frage, ob ich da richtig liege.

      Kommentar


      • #4
        Zitat von HeinrichK Beitrag anzeigen
        Dass intval() nicht prüft, weiß ich. Aber es filtert doch wohl aus einem String alles, was kein Integer ist - richtig?
        Falsch!
        PHP-Code:
        <?php
           
        echo intval("abc123def");
        ?>
        Ausgabe: 0
        Die Deutsche Rechtschreibung ist Freeware! Du darfst sie kostenlos nutzen, allerdings ist sie nicht Open Source, d.h. Du darfst sie nicht verändern oder in veränderter Form veröffentlichen.

        Kommentar


        • #5
          Was ist der Sinn von dem Code in #1? Nutze doch zB filter_input(), Prepared Statments. Das sonst alles da oben kommt mir irgenwie "unartig" vor. Ok ein reines White-Listen von GET Parametern ist ok falls es darum geht.

          Und du solltest die Verbindung besser in die Funktion übergeben (als Parameter) und nicht in der Funktion einen Include der DB-Daten machen, das ist auch nicht die Feine Art.. Wenn du das bei jeder Funktion, die die DB benötigt, so machst is das etwas sinnlos.

          http://php-de.github.io/#security

          LG
          Debugging: Finde DEINE Fehler selbst! | Gegen Probleme beim E-Mail-Versand | Sicheres Passwort-Hashing | Includes niemals ohne __DIR__
          PHP.de Wissenssammlung | Kein Support per PN

          Kommentar


          • #6
            Das ist wirklich ziemlich wirr, warum nicht einfach den Parameter in die Datenbank jagen und sehen ob etwas gefunden wird?

            Zur aktuellen Funktion:
            - Im SQL lieber DISTINCT nutzen anstatt zu gruppieren, semantische Korrektheit und so
            - warum das strip_tags() vor dem Funktionsaufruf? Damit nimmst du der Funktion ja schon eine Aufgabe ab, bzw. verschleierst Manipulationen bevor sie geprüft wird
            - ich würde wohl eher Rückgabewerte nutzen, macht die Funktion flexibler.. lässt dir auch andere Optionen offen wie du auf Falscheingaben reagieren kannst
            - wie schon gesagt, Verbindung per Parameter übergeben
            Relax, you're doing fine.
            RTFM | php.de Wissenssammlung | Datenbankindizes | Dateien in der DB?

            Kommentar


            • #7
              Oh schön - danke für die Tipps - da kann ich (nach dem Krimi) dran arbeiten. Vorab soviel:

              @uha, hausl
              Das mit intval() habe ich verstanden. Nützt aber immerhin, dass kein Schadcode reinkommt.
              Aber diese filter-Funktionen sind ja viel besser und variabler. Z.B.
              PHP-Code:
              $id filter_var($_GET['id'], FILTER_SANITIZE_NUMBER_INT); 
              @VPh
              Ich dachte immer, es geht gerade darum, Parameter nicht ungeprüft einfach "in die Datenbank zu jagen". Dort werden sie in der WHERE-Klausel zwar escaped, aber reicht das aus?

              Alles weitere spätere.

              Kommentar


              • #8
                PHP-Code:
                $d filter_var($_GET['id'], FILTER_SANITIZE_NUMBER_INT); 
                Das wirft eine Notice, wenn Parameter nicht vorhanden:

                Code:
                Notice Undefined index: id in ....
                Nutzt du hingegen das, dann nicht. Wenn Parameter nicht vorhanden, dann wird "nur" null zurückgegeben.

                PHP-Code:
                $e filter_input(INPUT_GET'id'FILTER_SANITIZE_NUMBER_INT);
                // keine Notice !!

                var_dump($e); // null 
                LG
                Debugging: Finde DEINE Fehler selbst! | Gegen Probleme beim E-Mail-Versand | Sicheres Passwort-Hashing | Includes niemals ohne __DIR__
                PHP.de Wissenssammlung | Kein Support per PN

                Kommentar


                • #9
                  Zitat von HeinrichK Beitrag anzeigen
                  Ich dachte immer, es geht gerade darum, Parameter nicht ungeprüft einfach "in die Datenbank zu jagen". Dort werden sie in der WHERE-Klausel zwar escaped, aber reicht das aus?
                  Ja, wenn man immer den Kontextwechsel beachtet reicht das.

                  Kommentar


                  • #10
                    Kontextwechsel
                    HeinrichK, siehe dazu:

                    Überblick: http://php-de.github.io/jumpto/kontextwechsel/
                    Ausführlicher Artikel: http://wiki.selfhtml.org/wiki/Artikel:Kontextwechsel
                    Debugging: Finde DEINE Fehler selbst! | Gegen Probleme beim E-Mail-Versand | Sicheres Passwort-Hashing | Includes niemals ohne __DIR__
                    PHP.de Wissenssammlung | Kein Support per PN

                    Kommentar


                    • #11
                      Ich gebe da VPh recht, würde ich genauso machen.
                      In Deinem Fall genügt ein Cast. intval liefert immer einen Integer, egal was der User eingegeben hat.
                      Damit einfach dieAbfrage an die DB senden und das wars.

                      Hat der User Mist eingegeben macht intval ne 0 draus und die Abfrage dürfte ins leere laufen.


                      Ich stufe mich ja gerne zurück, wenn das geht.
                      Benutzerkontrollzentrum -> Selbsteinschätzung
                      Competence-Center -> Enjoy the Informatrix
                      PHProcks!Einsteiger freundliche TutorialsPreComposed Packages

                      Kommentar


                      • #12
                        Also, ich fasse für mich mal zusammen.

                        Bei numerischen Werten aus GET-Parametern genügt ein Cast (nach Arne) mit intval() oder einer filter-Funktion (dann geht Schadcode schon ins Leere) und das Escapen in der WHERE-Klausel.

                        Bei anderen Werten ("um" ist nichtnumerisch) ist die Whitelist sinnvoll.

                        Das Thema "Kontextwechsel" (hausl) glaube ich, verstanden zu haben. Oder spricht mein Beispielcode dagegen?

                        Einige andere Tipps zur besseren Syntax werde ich noch bearbeiten.

                        Wenn niemand das Obige für falsch hält, könnte man den Thread schließen, oder?

                        Es kommen bestimmt noch weitere Fragen von mir..

                        Kommentar


                        • #13
                          Das Thema "Kontextwechsel" (hausl) glaube ich, verstanden zu haben. Oder spricht mein Beispielcode dagegen?
                          Also, kann sein, dass du das richtig verstanden hast. Nur ist eine Whitelist…

                          PHP-Code:
                          // Anti-SQL-Injection Parameter "um" (whitelist) 
                          …nicht der passende Weg, einen Kontextwechsel zu behandeln.

                          Ich schaffe es gerade leider nicht, dafür eine gute Erklärung zu finden. Vielleicht: Ein Kontextwechsel lässt sich gemeinhin allgemein lösen (durch zugehörige Escaping-Funktionen). Du „löst“ ihn aktuell nur für eine festgelegte Untermenge von Eingaben (die Whitelist). Das ist recht genau das, was unter anderem ich unter anderem hier schon erwähnt habe:

                          Kontextwechsel einfach immer dann mit der entsprechenden Funktion behandeln, wenn sie stattfinden. Von Überlegungen im Sinne von „hier muss ich beim Kontextwechsel nichts mehr machen, weil der Wert okay ist“ rate ich entschieden ab. Kontextwechsel einfach immer an passender Stelle behandeln. Dann brauchst du (oder andere Personen, die den Code lesen könnten) nicht darüber nachzudenken, ob an einer Stelle jetzt nichts getan werden muss oder ob du es nur vergessen hast.
                          - http://www.php.de/830653-post27.html

                          „Passende Stelle“ meint den Punkt, an dem du die Daten beispielsweise in einen SQL-Query-String einfügst.

                          Vielleicht ein Beispiel:

                          PHP-Code:
                          // Query
                          $query sprintf("SELECT * FROM users WHERE user='%s' AND password='%s'",
                                      
                          mysql_real_escape_string($user),
                                      
                          mysql_real_escape_string($password)); 
                          - Ist aus der Doku: http://php.net/manual/en/function.my...ape-string.php

                          Es ist völlig egal, was in den Strings $user und $password steht. Die Query wird immer syntaktisch korrekt sein.

                          Dein Whitelist-Ansatz wird zwar – wenn/falls nicht irgendwie Unsinn in die Liste gerät oder so – „korrekt funktionieren”, aber er ist das falsche Instrument für den Kontextwechsel.

                          Eine Whitelist kann an der Stelle (beim Einfügen oder Editieren von Daten) zusätzlich genutzt werden, um im Sinne der Geschäftslogik deiner Anwendung nur bestimmte Eingaben zuzulassen, aber das ist ein anderer Punkt und hat nichts mit der Behandlung von Kontextwechseln zu tun.

                          Kommentar


                          • #14
                            Hallo mermshaus,
                            Kontextwechsel einfach immer dann mit der entsprechenden Funktion behandeln, wenn sie stattfinden. Von Überlegungen im Sinne von „hier muss ich beim Kontextwechsel nichts mehr machen, weil der Wert okay ist“ rate ich entschieden ab.
                            Das mache ich auf alle Fälle. Jeder Wert wird bei seiner Verwendung im mysql-Statement escaped, z.B.
                            PHP-Code:
                            WHERE um'".mysql_real_escape_string($um)."' 
                            Ich verstehe jetzt deinen Beitrag (und andere) so, dass dies zur Sicherheit völlig ausreicht.

                            Meine Intention war es, mit der Whitelist eine vorausgehende Überprüfung durchzuführen. Wenn ich's richtig verstehe, ist dies dann überflüssig...?

                            Kommentar


                            • #15
                              Das sind 2 verschiedene Themen. Das eine ersetzt nicht das andere, teilweise bauen sie aufeinander auf.

                              Validierung (z.B. Whitelist) - prüft ob der Wert erlaubt/valide ist
                              Sicherheitsmaßnahmen (Escaping) - sorgt dafür, dass schädliche Eingaben 'entschärft' werden

                              Bei deiner Whitelist handelt es sich um die Einträge aus der Tabelle in der Datenbank, vollkommen ok. In deiner aktuellen Funktion umgehst du den Kontextwechsel, indem du erst die Whitelist in ein PHP-Array überträgst und dann nachsiehst, ob der übergebene Wert drinsteckt.
                              Durch beachten des Kontextwechsels zur Datenbank, Einsatz von Prepared Statements / korrektes quoten und escapen, musst du diesen Umweg über das PHP-Array nicht gehen.
                              Relax, you're doing fine.
                              RTFM | php.de Wissenssammlung | Datenbankindizes | Dateien in der DB?

                              Kommentar

                              Lädt...
                              X