Sicherheit include()

[punisher]

Hallo User,

hab gerade entdeckt das auf unserer Firmenseite folgendes include statement verwendet wird:

include($_GET['site']) 

Ich weiß dass das sehr unsicher ist, da jemand eine Seite mit php code von außerhalb aufrufen könnte. Reicht es allerdings wenn ich den Code wie folgt abändere?

include('./'.$_GET['site']) 

Grüße,
Punisher

[XGremliN]

auch wenn du dass so machst, kann in $_GET['site'] noch böswilliger Code stehen. Erstmal solltest du addslashes() verwenden und zweitens prüfen, ob auch die zu includende Datei existiert.

[jahlives]

Ein solcher include() sollte immer gegen eine Liste der erlaubten Dateien verglichen werden. Nur zu prüfen ob die Datei existiert reicht nicht, denn sonst könnte ein User auch Pw-Dateien auf deinem Server anzeigen.
Also ein Array mit erlaubten Dateien definieren und prüfen ob der $_GET Parameter in diesem erlaubten Array liegt.

Gruss

tobi

[wahsaga]

Re: Sicherheit include()

Original geschrieben von punisher
Reicht es allerdings wenn ich den Code wie folgt abändere?
include('./'.$_GET['site'])

Nein, natürlich nicht - dann übergebe ich ../datei, ../../datei etc., wenn ich in darüber liegende Verzeichnisse wechseln möchte.

[punisher]

Danke für die vielen Antworten. Das wechseln in ein höheres Verzeichnis hab ich nicht bedacht...

Gibt es denn eine möglichkeit, alle nur im momentanen bzw. einem bestimmten Verzeichnis zu suchen ohne das ich in ein anderes wechseln kann?

Ich weiß dass die Methode mit einer Liste der erlaubten Seiten wohl die sicherste ist, aber hierfür müsste ich noch einmal die komplette Struktur ändern, was ich möglichst vermeiden möchte.

Danke für Eure hilfe!
Punisher

[wahsaga]

Du hast einen dicken Fehler gefunden. Dass dessen saubere Behebung jetzt etwas mehr Aufwand bedeutet, solltest du akzeptieren - anstatt gleich zur nächsten Pfusch-"Lösung" zu greifen.

[punisher]

Original geschrieben von wahsaga
Du hast einen dicken Fehler gefunden. Dass dessen saubere Behebung jetzt etwas mehr Aufwand bedeutet, solltest du akzeptieren - anstatt gleich zur nächsten Pfusch-"Lösung" zu greifen.

Autsch ... Ohrfeige völlig zurecht

Nein, ich geb dir in dem Punkt vollkommen recht. Es ist nur so das wir im Moment ziemlich wenig Zeit für eine Umsetzung dieser Art haben. Deswegen würde ich das Loch jetzt für den Moment gerne Stopfen, zu gegebener Zeit dann aber RICHTIG schließen.

Andererseits muss ich dann meinem Chef auch wieder den höheren Verwaltungsaufwand irgendwie erklären. Immerhin will die Liste ja dann gepflegt werden. Wie setzt Ihr sowas um? Extra Datei mit einer Liste, oder in einer DB oder als Array?

Grüße Punisher

[Kropff]

Extra Datei mit einer Liste, oder in einer DB oder als Array?

hängt davon ab, wer die die liste pflegt und wie lang die ist. ich benutze für sowas meistens arrays

peter

[jahlives]

Ich würde jetzt eine Extra Datei machen, die du in ein Verzeichnis knallst auf das nur der Server zugriff hat
Den ganzen include() würde ich dann in eine header.php packen welche du in die jeweiligen Seiten einbaust

//Textdatei seiten.txt
/pfad/verzeichniss/datei.php
/anderer/pfad/daten.php

//header.php
$f = file('seiten.txt');
if(in_array($_GET['site'],$f) && file_exists($_SERVER['DOCUMENT_ROOT'].$_GET['site']){
    include($_GET['site']);
}else{
    die('Geh woanders spielen');
} 

Gruss

tobi

[punisher]

Also, danke noch mal für die vielen Antworten. Werde eure Ideen zu gegebenem Zeitpunkt berücksichtigen. Vorübergehend hätte ich jetzt folgende Lösung:

Ich überprüfe erst den Suchstring mit einem Regex ob er folgendem Format entspricht:
./produkte.htm oder ./produkte.html
Dann überprüfe ich, ob die Datei vorhanden ist ...

Kann mir einer sagen ob der folgende Regex alles außer dem oben geschriebenen ausschließt?
"/^(\.\/)[a-zA-Z0-9]+(\.htm|\.html)/";

Laut meinen Tests müsste es funktionieren, ein ../ im String wäre dann nicht mehr erlaubt ...

Danke noch mal für eure Hilfe!
Punisher

[closure]

Hi,
bei großen projekten kann man sich dann überlegen ob
man nicht lieber ein associatives array benutzt, damit geht der
lookup schneller und man kann gleichzeit noch cachen ob bereits
geprüft wurde ob die datei auch vorhanden ist. Dadurch spart
man sich dann wieder das file_exists.

greets

[PHP-Desaster]

du kannst, um das wechseln in höhere verzeichnisse auch einfach die zwei punkte ersetzen:

$_GET['site'] = str_replace( "../", "", $_GET['site'] );