Security Frage (XSS)

[TTlong]

Hallo,

ich beschäftige mich gerade mit der Sicherheit von PHP-Anwendungen und habe folgenden Code aus einem Buch:

CODE <?php
$whitelist = array ('a','b','c');
if(in_array(trim($_GET['skript']),$whitelist))
{
require($_GET['skript'].'.php');
}
else
{
die()
}
?>


Es geht hier darum, serverseitiges Cross-Site-Scripting durch URL-Injection zu verhindern.
Im obigen Beispiel sollen nur die dateien "a", "b" oder "c" eingebunden werden dürfen (eine whitelist), andernfalls die().

Es funktioniert aber so nicht ( die() ).

Es gibt natürlich die Dateien a.php etc.
Muss ich für 'skript noch irgendetwas einsetzen oder ist es dort an der richtigen Stelle?

Irgendwie stehe ich auf dem Schlauch.

 

[TTlong]

Hat keiner eine Idee?

 

[moeli]

was ist die Fehlermeldung?

und probier mal die();

 

[Jürgen Auer]

QUOTE (TTlong @ Sa 7.4.2007, 20:53)
CODE <?php
$whitelist = array ('a','b','c');
if(in_array(trim($_GET['skript']),$whitelist))
{
require($_GET['skript'].'.php');
}
else
{
die()
}
?>


Ich kenne mich mit PHP nicht wirklich aus, von daher schweige ich bei solchen Dingen lieber.

Auf die Entfernung fällt mir zu dem Code ein:

$_GET['skript'] vor der Prüfung einmal ausgeben lassen (mit umgebenden Dummy-Buchstaben), um zu sehen, was tatsächlich übergeben wird.

Eine Funktion ist nicht definiert (in_array?) Ist es legitim, require an so einer Stelle einzufügen?

Da die Adressierung auf die Datei relativ ist: Stimmt das? Sprich: Davor irgendeine Testüberprüfung rein, ob die Datei existiert bzw. sich einmal den aktuellen Pfad ausgeben lassen.

 

[G.P.]

Um diesen Angriff zu vermeiden, nutze ich immer folgende moeglichkeit.
Evtl. hilft es dir ja etwas

CODE
if($_GET['page'])
{
if(file_exists($root.'abc_'.$_GET['page'].'.php')) $show_page = $_GET['page'];
else $show_page = 'index';
}
else $show_page = 'index';

include($root.'abc_'.$show_page.'.php');



MfG
GP

 

[TTlong]

QUOTE (G.P. @ So 8.4.2007, 12:17) Um diesen Angriff zu vermeiden, nutze ich immer folgende moeglichkeit.
Evtl. hilft es dir ja etwas


CODE
if($_GET['page'])
{
if(file_exists($root.'abc_'.$_GET['page'].'.php')) $show_page = $_GET['page'];
else $show_page = 'index';
}
else $show_page = 'index';

include($root.'abc_'.$show_page.'.php');



MfG
GP

Mir geht es ja darum, die GET-Variable nicht ungeprüft zu nutzen. In deinem Fall sieht es irgendwie nicht so aus. Weiter nutze ich lieber "require" um Dateien einzubinden, der Sicherheit wegen.

die(); bringt eine Fehlermeldung (weiss grad nicht welche, mein lokaler Server läuft bei aktiver Internetverbindung nicht).

Schau mich mal im PHP-Forum um. Danke euch.

 

[G.P.]

in diesem Fall wird die Get Variable ueberprueft

Die Whitelist sind in diesem Fall alle mit "abc_" beginnenden php Dateien in einem vorher fest bestimmten Ordner, welcher per $root festgelegt wird.
Meiner Meinung nach eine recht sichere und sehr leichte Methode (ich lass mich aber gerne eines besseren belehren)

Ob du include oder require verwendest ist dabei erst einmal irrelevant. Ob require sicherer ist als include wage ich aber zu bezweifeln

MfG
GP

 

[Sascha Ahlers]

QUOTE (TTlong @ So 8.4.2007, 11:28) [...] Weiter nutze ich lieber "require" um Dateien einzubinden, der Sicherheit wegen. [...]

Das erklär mir bitte mal, warum das sicherer sein soll?
Ich würde hier eher ein include, als ein require, benutzen, besonders da ich mit PHP 3 angefangen habe:


QUOTE Anmerkung: Für frühere Versionen als PHP 4.0.2 gilt folgendes: require() wird immer versuchen die Zieldatei zu lesen, selbst wenn die Zeile in der die Anweisung steht, nie ausgeführt wird. Eine bedingte Anweisung hat keine Auswirkungen auf require(). Wenn jedoch die Zeile in der require() steht, nie ausgeführt wird, wird auch der Code der Zieldatei nie ausgeführt werden. Ähnliches gilt für Schleifenstrukturen, diese beeinflussen das Verhalten von require() nicht. Obwohl der Code, der in der Zieldatei enthalten ist, zur Schleife gehört, wird require() selbst nur einmal ausgeführt.

Quelle: http://www.php.net/require

 

[Sascha Ahlers]

QUOTE (G.P. @ So 8.4.2007, 12:05) in diesem Fall wird die Get Variable ueberprueft

Die Whitelist sind in diesem Fall alle mit "abc_" beginnenden php Dateien in einem vorher fest bestimmten Ordner, welcher per $root festgelegt wird.
[...]

Na na, da kann man draus ausbrechen, der Programmcode von Dir ist keinesfalls sicher! Auch wenn Du mit $root einen Ordner festlegest, gibt es die Möglichkeit mit /../ ggf. daraus auszubrechen. Auch könnten Exploits genutzt werden, welche Sonderzeichen oder andere Zeichenkodierungen nutzen.

 

[Joel]

QUOTE
in diesem Fall wird die Get Variable ueberprueft
Die Whitelist sind in diesem Fall alle mit "abc_" beginnenden php Dateien in einem vorher fest bestimmten Ordner, welcher per $root festgelegt wird.
Meiner Meinung nach eine recht sichere und sehr leichte Methode (ich lass mich aber gerne eines besseren belehren)

Ob du include oder require verwendest ist dabei erst einmal irrelevant. Ob require sicherer ist als include wage ich aber zu bezweifeln



Mal angenommen dein Script-Folder ($root) ist folgender:
http://www.domain.com/php/cms/scripts/

und dort hats Dateien mit den namen abc_dosomething.php und abc_xyz.php.

$_GET['page'] setze ich auf ../../user-uploads/abc_myupload.jpg oder sogar ../../user-uploads/abc_myupload.php (wenn das erlaubt ist). Das hat nichts mehr mit XSS zu tun, sondern ist ein schwerwiegendes Sicherheitsloch.

PS: Hab grad gesehen, dass "Sascha Ahlers" die Sache auch schon beantwortet hat

.

 

[Sascha Ahlers]

QUOTE (Joel @ So 8.4.2007, 12:16)Mal angenommen dein Script-Folder ($root) ist folgender:
http://www.domain.com/php/cms/scripts/

Das ist sogar noch schlimmer, da ist auch noch DNS-Spoofing möglich.

Ich würde es eher so schreiben:

QUOTE <?php

$root = '/var/www/kunde/web123/ordner/';
$whitelist = array ('a', 'b', 'c'
$skript = '';

if ( isset($_GET['skript']) ) {
   $skript = trim(rawurldecode($_GET['skript']));
}

if ( in_array($skript, $whitelist) ) {
   include($root . $skript . '.php'
} else {
   exit();
}

?>


Alternativ kann die $_GET['skript'] auch wieder in sich gespeichert werden, wenn man die Isolation zu anderen Variablenen beibehalten möchte, dazu muss diese aber ggf. deklariert werden, wenn dies noch nicht geschehn sein sollte.

 

[G.P.]

Ok, ich habe vergessen zu erwaehnen das nur Dateien aus dem bestimmten Ordner geladen werden koenne, daher ist eine moeglichkeit ala ../ ausgeschlossen.
Aber das mit den Sonderzeichen ist richtig. Danke fuer den Hinweis. Ich werd es gleich einmal ueberarbeiten.

MfG
GP

 

[Sascha Ahlers]

QUOTE (G.P. @ So 8.4.2007, 13:01) Ok, ich habe vergessen zu erwaehnen das nur Dateien aus dem bestimmten Ordner geladen werden koenne, daher ist eine moeglichkeit ala ../ ausgeschlossen. [...]

Das hast Du doch erwähnt, es findet aber keine Filterung statt, darum kann sowas wohl funktionieren, wenn es nicht von den Servereinstellungen abgefangen wird.

 

[TTlong]

QUOTE (Sascha Ahlers @ So 8.4.2007, 13:08)
QUOTE (TTlong @ So 8.4.2007, 11:28) [...] Weiter nutze ich lieber "require" um Dateien einzubinden, der Sicherheit wegen. [...]

Das erklär mir bitte mal, warum das sicherer sein soll?
Ich würde hier eher ein include, als ein require, benutzen, besonders da ich mit PHP 3 angefangen habe:

Weil "require" sofort das gesamte Script abbricht und nicht wie "include" nur den fehlerhaften Teil.

Werd deine Ausführung gleich mal mal testen, danke!

 

[MarkusH]

Hallo,

ich finde die Ursprungsversion ist doch ziemlich gut. Was kann denn da passieren bzw fälschlicherweise oder boshafterweise eingebunden werden? Eine Whitelist ist ja so ziemlich das schärfste was geht; zumindest kurz nach einem (Parameter-Mapping).

Die Version von GP benutze ich so ähnlich auch, allerdings mit Sonderzeichenmaskierung
CODE $root='/pfad/zum/file/';
$fileToTry=quotemeta(trim($_GET['file']));
$file=file_exists($root.$fileToTry.'.php')?$fileToTry:'index';
include($root.$fileToTry.'.php');


@Sascha: Welchen Sinn macht es, den Get-String erst nochmal zu encoden und ihn dann mit einer Whitelist abzugleichen? Reicht nicht der Abgleich mit der Whitelist? Wenn der übergebene Wert "a" lautet, und "a" in der Whitelist zu finden ist und somit "a.php" eingebunden wird, was kann dann noch anderes im Get-String stehen, das gefährlich wäre?

QUOTE ("Sascha&quot<?php

$root = '/var/www/kunde/web123/ordner/';
$whitelist = array ('a', 'b', 'c'
$skript = '';

if ( isset($_GET['skript']) ) {
$skript = trim(rawurlencode($_GET['skript']));
}

if ( in_array($skript, $whitelist) ) {
include($root . $skript . '.php'
} else {
exit();
}

?>

 

[TTlong]

QUOTE (MarkusH @ So 8.4.2007, 16:14) Hallo,

ich finde die Ursprungsversion ist doch ziemlich gut. Was kann denn da passieren bzw fälschlicherweise oder boshafterweise eingebunden werden? Eine Whitelist ist ja so ziemlich das schärfste was geht; zumindest kurz nach einem (Parameter-Mapping).

Nur sie funktioniert so nicht??!! Leider!

 

[Sascha Ahlers]

QUOTE (MarkusH @ So 8.4.2007, 15:14)
[...]
@Sascha: Welchen Sinn macht es, den Get-String erst nochmal zu encoden und ihn dann mit einer Whitelist abzugleichen? Reicht nicht der Abgleich mit der Whitelist? Wenn der übergebene Wert "a" lautet, und "a" in der Whitelist zu finden ist und somit "a.php" eingebunden wird, was kann dann noch anderes im Get-String stehen, das gefährlich wäre?
[...]


Im Grunde reicht es aus, das muss aber nicht urlrawencode lauten, sondern eigentlich urlrawdecode, da habe ich mich leicht versehen (hab's mal eben geändert). Veraussetzung ist natürlich, dass keine Exploits innerhalb PHP selber vorhanden sind. Grundsätzlich finde ich aber solche Ansätze zum Einbinden von Dateien schrecklich, ich schreibe Dateieinbindungen lieber nicht mittels übergebener Variable nieder. Lieber in dieser Form:


CODE switch ( $mode ) {
case 'edit': include('./edit.php'); break;
default: include('./list.php');
}




QUOTE (MarkusH @ So 8.4.2007, 15:14)[...]
Die Version von GP benutze ich so ähnlich auch, allerdings mit Sonderzeichenmaskierung

CODE $root='/pfad/zum/file/';
$fileToTry=quotemeta(trim($_GET['file']));
$file=file_exists($root.$fileToTry.'.php')?$fileToTry:'index';
include($root.$fileToTry.'.php');

[...]

Von GPs Version würde ich aber ausdrücklich abraten, entweder direkt darauf achten, dass wirklich nur Dateien in den Ordner aufgerufen werden können oder mit einer Whitelist arbeiten. Es ist sonst sehr einfach möglich daraus auszubrechen. Zu dem Code müsste noch eine Prüfroutine hinzugefügt werden, die darauf achtet, dass niemand versucht aus den Ordner auszubrechen (mittel ../).
Eine Prüfung von file_exists ist schön, ist aber nicht ganz sauber, außerdem lässt sich das einfacher regulieren.
Auf Wunsch schriebe ich dies auch mal etwas ausführlicher in mein Blog nieder, wie man dieses herausspringen verhindern kann.

 

[Jürgen Auer]

QUOTE (Sascha Ahlers @ So 8.4.2007, 17:20)Zu dem Code müsste noch eine Prüfroutine hinzugefügt werden, die darauf achtet, dass niemand versucht aus den Ordner auszubrechen (mittel ../).

Von solchen Codes würde ich grundsätzlich die Finger lassen.

Bei diesem Problem - relative Adressen in erlaubte absolute Adressen umrechnen - haben sich bereits einige größere Firmen die Finger dran verbrannt. Bei Microsoft gab es raffinierteste Hacks, die bsp. Unicode-Mehrfachcodierungen von ../ nutzten, die von der aufgerufenen Systemfunktion automatisch korrekt decodiert werden, bei denen die Suche nach ../ aber erfolglos bleibt.

Sprich: Man muß alle Fälle kennen, wie eine Systemfunktion wie 'include' relative Adressen entgegennehmen kann. Und da gibt es in der Regel mehr als nur das, was offenkundig ist. Und übersieht man eine Möglichkeit, dann kann das eine Lücke erzeugen.

Sprich: Whitelist und sonst nichts.

 

[Sascha Ahlers]

QUOTE (jAuer @ So 8.4.2007, 17:56) [...] Von solchen Codes würde ich grundsätzlich die Finger lassen. [...]

Drum schriebe ich ja auch:


QUOTE (Sascha Ahlers @ So 8.4.2007, 17:20)[...] Grundsätzlich finde ich aber solche Ansätze zum Einbinden von Dateien schrecklich, ich schreibe Dateieinbindungen lieber nicht mittels übergebener Variable nieder. Lieber in dieser Form: [...]