Frage zur PHP-Sicherheit

Dann werde ich das Include so verwenden.
Was ich noch eingefügt habe ist basename, was den Dateinamen extrahiert.

PHP:
if(!isset($_GET["page"])) {  

$page = "news";  

} else {  

$page = preg_replace( "/[^a-z0-9]/im", "", $_GET["page"]); 
$page = basename($page);

  } 


$whitelist = array("news"=>"news", "archiv"=>"news", "search"=>"news", "inhalt"=>"inhalt");    

if (!empty($page) && isset($whitelist["$page"])) {   


$file = "./".$whitelist["$page"]."/".$page.".php";  


if(file_exists($file)) { include $file; }  

   }else{ 

if(file_exists("./inhalt/fehler.php")) { include "./inhalt/fehler.php"; } 

} 

 }
 
Hallihallo,

Der Teil hier funktioniert aber so nicht, da bekomm ich ne Fehlermeldung.

Parse error: syntax error, unexpected T_ARRAY, expecting T_STRING or T_VARIABLE or '$' index.php on line 640

Der Syntaxfehler tut mir leid. Aber es war halt auch schon spät und ich hatte den Code auch nicht näher getestet. Dank Gumbos Korrektur sieht es jetzt auch so aus, wie ich es mir gedacht hatte.

Nun nochmal zu einem angesprochenen Problem:
Meiner Meinung nach ist das in_array sicherer da ja genau überprüft wird was eingegeben wurde z.b. news.

Beim 2ten Codeschnipsel unten wird doch nur überprüft ob page nicht leer ist und ob die whitelist existiert aber nicht genau was drinsteht.
Nicht ganz. Würde da nur isset($whitelist) stehen, wäre das die Überprüfung, ob die whitelist existiert. Aber isset($whitelist[$page]) prüft ob im Array Whitelist ein Schlüssel mit dem Wert von $page enthalten ist. Mehr Informationen kannst du mit in_array() auch nicht feststellen. Der wesentliche Unterschied zwischen beiden Funktionen ist in diesem Fall, dass du mit in_array() auf das Vorhandensein eines Wertes im Array prüfst und mit isset($array[$key]) auf das Vorhandensein des Schlüssels (Index) mit dem du dann auf den Wert zugreifen kannst.
Tatsächlich genügt die Variante mit in_array() völlig und wäre auch meine bevorzugte Vorgehensweise. Allerdings wurde hier nach einer Möglichkeit gefragt, auf Dateien in verschiedenen Verzeichnissen zu verlinken. Und dafür war mein Vorgehen eine Möglichkeit.
Zur Erinnerung der Aufbau des Arrays am Anfang, wie ihn ein print_r($whitelist) ausgeben würde:
Code:
Array
(
    [0] => news
    [1] => archiv
    [2] => search
    [3] => guestbook
    [4] => inhalt
)

Allgemein könnte man es auch so darstellen: array($datei1, $datei2, ...) Das Array benhaltet die Namen der Dateien (ohne die Erweiterung .php). Mein Vorschlag war nun ein Array zu erzeugen, dass den Aufbau hat array($datei => $verzeichnis1, $datei2 => $verzeichnis2, ...). Wenn du also hier eine Datei und ein Verzeichnis in die whitelist eingibst, dann - und nur dann - wird die if-Abfrage den Verarbeitungsteil mit dem include ausführen lassen. Mit einem ungültigen Wert (nicht in der whitelist enthalten) kommst du bei dieser Variante genauso wenig weiter wie mit dem in_array(). Aber hier kannst du nun mit dem Wert in $page aus der whitelist nicht nur die Existitenz eines erlaubten Dateinamens sondern auch gleich noch sein Verzeichnis dazu erhalten. Das ist mit dem ursprünglichen Array nicht möglich und bei dem veränderten Array kann in_array() leider nicht mehr die gewünschte Funktionalität stellen.
Natürlich könnte man auch das Array anders aufbauen: array($verzeichnis1 => $datei1, $verzeichnis2 => $datei2, ...). Dann könnten wir wieder in_array() einsetzen und außerdem ist dieser Aufbau des Arrays auch viel nahe liegender, um nicht zu sagen logischer. Allerdings praktisch ist er nicht. Um das Verzeichnis zu einer Datei zu ermitteln müsste man dann viel mehr Aufwand betreiben und beispielsweise durch eine Schleife iterieren, um an die Information zu kommen.
PHP:
$page = ...; // Ermitteln des Wertes $page wurde bereits besprochen. Ich setze ihn hier mal voraus, um mich auf das Wesentliche zu konzentrieren.
$dir = ""; // Wenigstens sinnvoll initialisieren
foreach ($whitelist as $directory => $file)
{
if ($page == $file)
{
$dir = $directory;
break;
}
}
Da ist die Anweisung
PHP:
$dir = $whitelist[$page];
doch sehr viel angenehmer.

Erschwerend kommt hinzu, wenn du mehrere Dateien im selben Verzeichnis hast, funktioniert das auch nicht mehr, da die Verzeichnisse ja hier als Schlüssel im Array fungieren und diese müssen eindeutig sein. Man könnte dann zwar mit einem mehrdimensionalen Array arbeiten. Aber was haben wir davon? in_array() wird dann wieder nicht funktionieren und die eben angesprochene Schleife müsste auch noch etwas ausgebaut werden.

Zusammenfassend: in_array() ist gut und funktioniert, solange das Verzeichnis immer das gleiche ist, oder auf einem anderen (noch nicht besprochenen Weg) bestimmt wird. Wenn das Verzeichnis von Relevanz ist, dann gibt es sicher mehrere Möglichkeiten. Eine habe ich vorgestellt. Die funktioniert, ist nicht sehr aufwendig und bedeutet auch nur einen minimalen Änderungsaufwand am bestehenden Code. Beide Varianten (in_array() oder isset()) erlauben durch die whitelist nur das Inkludieren von Dateien, die durch den Entwickler explizit aufgezählt werden müssen, was eine hohe Sicherhheit verspicht.

H I H und Gruß
Mitleser.



p.s. Ich persönlich setze eigentlich lieber include_once() statt include() ein. Vermeidet Fehler.

Was ich noch eingefügt habe ist basename, was den Dateinamen extrahiert.

Kannst du machen. Die Idee ist im Prinzip nicht schlecht, da du damit eine weitere Problemstelle reduzierst. Es ist nur fraglich, ob es auch soviel bringt.

Wenn du URLs wie ....?index.php&page=public/daten/news zu erwarten hast, also welche in denen dein Parameter als Pfad angeben ist, von dem du ja nur den Dateinamen haben willst, dann wird dir daraus dann natürlich mit basename() nur noch der Wert »news« aus dem Parameter genommen.
Das hilft dir natürlich weiter. Ich vermute aber, dass du i.d.R. in der Zeile nichts weiter erreichst. Bei einer URL wie ....?index.php&page=news wird $page den Wert »news« beinhalten, egal ob du nun noch basename() darauf anwendest oder nicht.

Der Aufruf schadet nicht und hilft, also laß ihn drin. Allerdings wird er selten bis gar nicht greifen.

Nochmal Grüße und viel Spaß
Mitleser
 
Hier nun das fertige Include Script :)

PHP:
if(!isset($_GET["page"])) { 

$page = "news"; 

} else { 

$page = preg_replace( "/[^a-z0-9]/im", "", $_GET["page"]);
$page = basename($page);

  }

$whitelist = array("seite1"=>"dir1", "seite2"=>"dir1", "seite3"=>"dir2");

if (!empty($page) && isset($whitelist["$page"])) { 

$file = "./".$whitelist["$page"]."/".$page.".php";   

if(file_exists($file)) { include_once $file; } 

   }else{

if(file_exists("./pfad/fehler.php")) { include_once "./pfad/fehler.php"; }

}

Ich danke nochmal allen die dazu beigetragen haben solch ein Sicheres und auch vielseitiges Include Script zusammenzutragen, und denke es wird einigen weiterhelfen die so etwas in der Art suchen :-)
 
Da kann man noch immer einiges verbessern.

Was ist z. B. wenn diese Bedingung fehl schlägt?
PHP:
if(file_exists($file)) { include_once $file; }
Dann wird keine Seite geladen, auch nicht die Fehlerseite.

Hier ein Beispiel.

PHP:
$page = "news"; 
$whitelist = array("news"=>"", "seite1"=>"dir1/", "seite2"=>"dir1/", "seite3"=>"dir2/");

if(isset($_GET["page"]) && !empty($_GET["page"])) { 
    $page = preg_replace( "/[^a-z0-9]/im", "", $_GET["page"]);
    $page = basename($page);
}

if (isset($whitelist[$page])) { 
    $file = "./".$whitelist[$page].$page.".php";
}

if(file_exists($file)) {
    include_once $file;
}
elseif(file_exists("./pfad/fehler.php")) {
    include_once "./pfad/fehler.php";
}
else {
    die( 'schwerer fehler' );
    /* immerhin gibts nichtmal die fehlerseite.. */
}

Nur so als Anregung und Denkanstoss, wo man überall noch am Rädchen drehen könnte. ;)
 
Zurück