Was haltet ihr von dieser Includemethode?

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

  • Was haltet ihr von dieser Includemethode?

    Hallo

    Es geht - ganz klassisch - darum, über die index.php eine Seite zu includen, die über die Url entgegengenommen wird (also z.B. index.php?site=news).

    Dazu habe ich folgendes geschrieben:

    PHP-Code:
    if (!preg_match("#^[A-Za-z0-9]$#"$_GET['page'])) {
    die();
    } else {
    $site 'includes/' $_GET['page'] . '.php';
      if (
    file_exists($site))
      {
            include(
    $site);
      }

    Was haltet ihr davon, vor allem Sicherheitstechnisch? Es wurde mir gesagt, dass es verdammt unsicher ist, mit einer Usereingabe DIREKT auf das Dateisystem zu zielen. Daraufhin hab ich noch den Regexteil hinzugefügt.

    Also:

    - Der Parameter aus der URL darf nur A-Za-z0-9 enthalten
    - die Datei muss auf PHP enden
    - die Datei muss im includes-Ordner stecken

    Der Parameter wird ja regelrecht in den Pfad gepresst.
    Ich hätte das eigentlich für verdammt sicher gehalten, aber werde gerne eines besseren belehrt.

  • #2
    Passt schon. Unsicher wirds erst, wenn jemand einen Bug in preg_match() entdeckt.
    Langsam ist es natürlich jetzt schon und auch nicht besonders sumafreundlich.

    Kommentar


    • #3
      Warum nicht fest definieren über eine switch?

      Ansich kann er somit alle Dateien aufrufen, die existieren. Egal, ob man sie sehen soll, oder nicht.

      mfg

      Kommentar


      • #4
        Hallo, danke schonmal

        Passt schon. Unsicher wirds erst, wenn jemand einen Bug in preg_match() entdeckt.
        Langsam ist es natürlich jetzt schon und auch nicht besonders sumafreundlich.
        Ich nehme mal an, file_exists ist der Flaschenhals? bei so Sachen fehlt mir einfach dieses Wissen, aber "langsam" ist ja auch manchmal subjektiv.

        Aber warum ist es nicht suchmaschinenfreundlich? (ich nehme mal an, dass "suma" Suchmaschine bedeutet ).
        Da die Suma den PHP-Code nicht zu gesicht bekommt, ist es doch genauso (wenig) sumafreundlich wie alle anderen Varianten, die "index.php?site=news" verarbeiten können
        Am Ende gibt diese URL doch nur den HTML-Code der News aus, genauso wie alle anderen Möglichkeiten.


        Warum nicht fest definieren über eine switch?
        Das ist meine bisher verwendete Methode, von der ich mich nun trennen wollte. Es ist einfach verdammt umständlich, jedes mal bei einer neuen Seite, die angelegt werden soll, den switch zu erweitern und wieder hochzuladen - statt einfach nur die neue Datei hochzuladen.


        Ansich kann er somit alle Dateien aufrufen, die existieren. Egal, ob man sie sehen soll, oder nicht.
        Es kann nur includet werden, was in einem speziell dafür angelegten Ordner ist (in dem Fall "includes").

        Kommentar


        • #5
          Ich nehme mal an, file_exists ist der Flaschenhals? bei so Sachen fehlt mir einfach dieses Wissen, aber "langsam" ist ja auch manchmal subjektiv.
          Nein, das preg_match! Reguläre Ausdrücke sind (fast) immer langsamer als die einfachen String-Funktionen.

          Aber warum ist es nicht suchmaschinenfreundlich?
          Denke, onemorenerd meint damit, dass du immer über die index.php gehst. Ein mod_rewrite in der Form:
          Code:
          http://www.example.com/news
          wäre sicher nicht verkehrt.

          Das du bei nicht-übereinstimmen des Regexp einfach stirbst ist ganz und gar nicht Sinnvoll! Da wäre eine Anzeige der Startseite oder einer 404 mit Suchfeld sinnvoller.

          Kommentar


          • #6
            Original geschrieben von PHP-Desaster
            Nein, das preg_match! Reguläre Ausdrücke sind (fast) immer langsamer als die einfachen String-Funktionen.

            Okay. Dann verwende ich am besten ctype_alnum? Dürfte schneller sein.
            Habe regex verwendet, da ich damit bei Formularen immer whitelisting betreibe und es deshalb gewohnt war.

            Original geschrieben von PHP-Desaster
            Denke, onemorenerd meint damit, dass du immer über die index.php gehst. Ein mod_rewrite in der Form:
            Code:
            http://www.example.com/news
            wäre sicher nicht verkehrt.

            mod_rewrite wird kommen. Aber über index.php zu gehen ist ja praktisch das Fundament der ganzen Sache, mod_rewrite schreibts ja praktisch nur um?

            Original geschrieben von PHP-Desaster
            Das du bei nicht-übereinstimmen des Regexp einfach stirbst ist ganz und gar nicht Sinnvoll! Da wäre eine Anzeige der Startseite oder einer 404 mit Suchfeld sinnvoller.
            Habe da nur ein die() geschrieben, um Codezeilen im Beitrag zu sparen. Eigentlich inkludiere ich ne 404-Seite, so dass keiner auf die Idee kommt, dass die Datei überhaupt existiert.
            Zuletzt geändert von INC.; 15.08.2008, 12:58.

            Kommentar


            • #7
              Okay. Dann verwende ich am besten ctype_alnum? Dürfte schneller sein.
              Zum Beispiel. Eigentlich sollte eine Kombination aus basename und file_exists schon genügen.

              mod_rewrite wird kommen. Aber über index.php zu gehen ist ja praktisch das Fundament der ganzen Sache, mod_rewrite schreibts ja praktisch nur um?
              Jep.

              Kommentar


              • #8
                PHP-Code:
                if(isset($_GET['page'])){
                  
                $file basename($_GET['page']);
                  if(
                file_exists("includes/{$file}.php")){
                    include(
                "includes/{$file}.php");
                  }else{
                   
                //hier kannst du alternativ einen 404-er werfen
                    
                die('Seite nicht gefunden');
                  }
                }else{
                  
                //Hauptseite anzeigen

                RegExp brauchst du imho ned. basename() sollte dafür sorgen dass keine ../ und ./ im Dateinamen diese Operation überleben
                Gutes Tutorial | PHP Manual | MySql Manual | PHP FAQ | Apache | Suchfunktion für eigene Seiten

                [color=red]"An error does not become truth by reason of multiplied propagation, nor does truth become error because nobody sees it."[/color]
                Mohandas Karamchand Gandhi (Mahatma Gandhi) (Source)

                Kommentar


                • #9
                  Original geschrieben von jahlives
                  PHP-Code:
                  if(isset($_GET['page'])){
                    
                  $file basename($_GET['page']);
                    if(
                  file_exists("includes/{$file}.php")){
                      include(
                  "includes/{$file}.php");
                    }else{
                     
                  //hier kannst du alternativ einen 404-er werfen
                      
                  die('Seite nicht gefunden');
                    }
                  }else{
                    
                  //Hauptseite anzeigen

                  RegExp brauchst du imho ned. basename() sollte dafür sorgen dass keine ../ und ./ im Dateinamen diese Operation überleben
                  include("includes/{$file}.php"); was ist das den für eine schreibweise? meine natürlich die {}
                  Gruß
                  Uzu

                  private Homepage

                  Kommentar


                  • #10
                    was ist das den für eine schreibweise?
                    Siehe Handbuch: http://www.php.net/manual/de/language.types.string.php
                    Wir werden alle sterben

                    Kommentar


                    • #11
                      was ist das den für eine schreibweise?
                      Vars sollten in " und " auch ohne {} gefunden werden. Aber z.B. in der IDE erhöht dieses Schreibweise die Lesbarkeit enorm, denn so werden die Vars auch als solche dargestellt und nicht einfach als Teil des Strings
                      Und hier finde ich {} auch nötig
                      PHP-Code:
                      //dat gibt nen Fehler
                      echo "A banana is $fruits['banana'] and tastes great";
                      //und dat ned
                      echo "A banana is {$fruits['banana']} and tastes great";
                      //und hier müssen sie sein
                      $fruits 'banana';
                      echo 
                      "{$fruits}tastes great";
                      //ohne { würde PHP nach einer Var $fruitstastes suchen und nicht finden 
                      Zuletzt geändert von jahlives; 15.08.2008, 13:53.
                      Gutes Tutorial | PHP Manual | MySql Manual | PHP FAQ | Apache | Suchfunktion für eigene Seiten

                      [color=red]"An error does not become truth by reason of multiplied propagation, nor does truth become error because nobody sees it."[/color]
                      Mohandas Karamchand Gandhi (Mahatma Gandhi) (Source)

                      Kommentar


                      • #12
                        Original geschrieben von jahlives
                        Vars sollten in " und " auch ohne {} gefunden werden. Aber z.B. in der IDE erhöht dieses Schreibweise die Lesbarkeit enorm, denn so werden die Vars auch als solche dargestellt und nicht einfach als Teil des Strings
                        Und hier finde ich {} auch nötig
                        Zend Studio stellt es auch ohne {} als Variable dar.

                        Kommentar


                        • #13
                          Okay, danke

                          Das mit dem basename versteh ich noch nicht so ganz.
                          Laut manual extrahiert die Funktion ja aus einem gegebenen Pfad einen Dateinamen.

                          Nun übertragt get ja sowas wie "news" (zumindest im besten Fall), aber vielleicht auch einen Pfad wenn der Angreifer will, oder vielleicht auch einen Code um einen eventuellen Bug in basename ausnutzen zu können (zu letztem: ka ob das möglich ist, aber mal angenommen).

                          Auf php.net kann man aber imho nirgends lesen, dass auch garantiert alle slashes entfernt werden. Da man ja nie weiß, was der User eingibt, sollte es ja eher heißen: basename _versucht_ den Dateinamen zu extrahieren. Und selbst wenn alle slashes entfernt werden, sind ja immernoch alle anderen Zeichen im String enthalten, die evt. was böses ausrichten könnten (was auch immer).

                          Bei ctype_alnum würde halt schonmal ordentlich im voraus gefiltert werden.

                          (Was macht basename eigentlich exakt? Sucht es immer das letzte Wort ohne slash am Ende eines Strings?)


                          edit, off topic:
                          @ jahlives, wäre statt echo "A banana is {$fruits['banana']} and tastes great"; nicht sowas wie print_f("A banana is %s and tastes great", $fruits['banana']); besser?
                          Zuletzt geändert von INC.; 15.08.2008, 15:03.

                          Kommentar


                          • #14
                            Also wenn ich basename('news') mache kriege ich wie gewünscht news raus. Egal wieviele ./ und ../ ich davor anhänge.
                            Jegliche Pfadangabe wird entfernt.
                            Und selbst wenn alle slashes entfernt werden, sind ja immernoch alle anderen Zeichen im String enthalten, die evt. was böses ausrichten könnten (was auch immer).
                            Das wichtigste beim include eines GET Parameters ist es alle Pfadangaben rauszuhauen und das macht basename() sicher. Die anderen Zeichen sind erstmal nicht so tragisch, da du ja zusätzlich prüfst ob die Datei in deinem vorgegebenen Verzeichnis existiert. Oder welche "gefährlichen" Zeichen meinst du sonst?
                            @ jahlives, wäre statt echo "A banana is {$fruits['banana']} and tastes great"; nicht sowas wie print_f("A banana is %s and tastes great", $fruits['banana']); besser?
                            imho Geschmackssache
                            Gutes Tutorial | PHP Manual | MySql Manual | PHP FAQ | Apache | Suchfunktion für eigene Seiten

                            [color=red]"An error does not become truth by reason of multiplied propagation, nor does truth become error because nobody sees it."[/color]
                            Mohandas Karamchand Gandhi (Mahatma Gandhi) (Source)

                            Kommentar


                            • #15
                              Original geschrieben von jahlives


                              Das wichtigste beim include eines GET Parameters ist es alle Pfadangaben rauszuhauen und das macht basename() sicher. Die anderen Zeichen sind erstmal nicht so tragisch, da du ja zusätzlich prüfst ob die Datei in deinem vorgegebenen Verzeichnis existiert. Oder welche "gefährlichen" Zeichen meinst du sonst?
                              Naja, bin da Laie, keine Ahnung, aber irgendetwas, was die Seite oder basename()/file_exists aus dem Takt bringen könnte (bei einem evt. Exploit oder Bug). Nur mit Buchstaben und Zahlen wäre das eher unmöglich.

                              Kommentar

                              Lädt...
                              X