Sicherheit - Hochgeladene XML-Datei parsen

[peter]

Hallo,

habe folgendes vor:

-Der User kann eine GPX-Datei (http://www.topografix.com/gpx.asp) zum Server schicken.

-Die hochgeladene Datei wird mit PHP geparst und die entsprechende Ausgabe erzeugt.

-Der User bekommt seine GPX-Datei dann auf einer Karte angezeigt.

Das funktioniert auch soweit, nur stellt sich mir die Frage, welche Sicherheitsrisiken dabei auftreten und wlche Maßnahmen ich ergreifen sollte.

Für Hilfe wäre ich euch dankbar.

gruß
peter

[dedlfix]

echo $begrüßung;

-Der User kann eine GPX-Datei (http://www.topografix.com/gpx.asp) zum Server schicken.
-Die hochgeladene Datei wird mit PHP geparst und die entsprechende Ausgabe erzeugt.
-Der User bekommt seine GPX-Datei dann auf einer Karte angezeigt.
Das funktioniert auch soweit, nur stellt sich mir die Frage, welche Sicherheitsrisiken dabei auftreten und wlche Maßnahmen ich ergreifen sollte.

Das kommt darauf an, wie genau die Umsetzung erfolgt ist. Womit wird die Datei geparst? Mit eigenem Code oder unter Verwendung einer der mitgelieferten XML-Parser? Wie reagiert das Programm auf Parser-Fehler, die ja bei ungültigen Dateien auftreten werden? Wie reagiert es auf gültige Syntax aber unsinnige Werte, wie beispielsweise außerhalb der Karte liegende Parameter? Wenn die Datei syntaktisch korrekt aber viel zu groß ist, wie reagiert das Script oder PHP oder der Server auf die übermäßige Ressourcenauslastung? Wie ist sichergestellt, dass im Code, den man nicht selbst geschrieben hat (sprich: in PHP selbt), Fehler beseitigt werden, wenn welche bekannt werden?

echo "$verabschiedung $name";

[peter]

Hallo dedlfix,

vielen Dank für Deine Antwort, die Fragen haben mich zumindest schon mal auf einige Punkte aufmerksam gemacht, über die man mal nachdenken sollte.

Das kommt darauf an, wie genau die Umsetzung erfolgt ist. Womit wird die Datei geparst? Mit eigenem Code oder unter Verwendung einer der mitgelieferten XML-Parser? Wie reagiert das Programm auf Parser-Fehler, die ja bei ungültigen Dateien auftreten werden? Wie reagiert es auf gültige Syntax aber unsinnige Werte, wie beispielsweise außerhalb der Karte liegende Parameter? Wenn die Datei syntaktisch korrekt aber viel zu groß ist, wie reagiert das Script oder PHP oder der Server auf die übermäßige Ressourcenauslastung? Wie ist sichergestellt, dass im Code, den man nicht selbst geschrieben hat (sprich: in PHP selbt), Fehler beseitigt werden, wenn welche bekannt werden?

Ich parse das Ganze mit eigenem Skript, hier mal der vereinfachte Code:

  
<form action="gpxup.php" method="post" enctype="multipart/form-data">  
  
GPX-Datei : <input size="8" name="uploaddatei" type="file" id="datei">  
<input value="Auswerten" type="submit" >  
  
</form>  
  
<?php  

  
if(isset($_FILES['uploaddatei']))  
    {  
    if($_FILES['uploaddatei']['error']=='UPLOAD_ERR_OK')  
        {  
        $DATEI=$_FILES['uploaddatei']['tmp_name'];  
  
  
        $parser=xml_parser_create();  
  
  
        $track=array();  
        $tracksegment;  
  
  
            function startElement($parser, $element, $attribute)  
                {  
                $element = strtolower($element);  
                global $track;  
                global $tracksegment;  
  
                    if ($element=="trkseg")  
                        {  
                        $tracksegment=array();  
                        array_push($track,$tracksegment);  
                        }  
  
                    if ($element=="trkpt")  
                        {  
                        $attributes=array();  
  
                            foreach($attribute as $key => $wert)  
                                {  
                                $key = strtolower($key);  
                                $attributes[$key]=$wert;  
                                }  
  
                        $trackpoint=array();  
  
                        $trackpoint[0]=$attributes['lon'];  
                        $trackpoint[1]=$attributes['lat'];  
  
                        array_push($tracksegment,$trackpoint);  
                        }  
                }  
  
  
            function endElement($xmlparser, $element)  
                {  
                }  
  
  
        xml_set_element_handler($parser, "startElement", "endElement");  
  
        $gpxdatei=fopen($DATEI,'r');  
  
            while ($data = fread($gpxdatei, 4096))  
                {  
                xml_parse($parser, $data, feof($gpxdatei));  
                }  
  
        xml_parser_free($parser);  
        }  
    }  

?>

Sollte vielleicht auf jeden Fall mal die Größe der hochladbaren Datei begrenzen, da muß ich mal schaun, wie man das macht.
Probleme sollte es da eigentlich nicht geben, so ne GPX-Datei is ja nicht so ewig groß, wenn der Track nicht gerade 10 mal um die Welt läuft oder so.

Erlaubte max. Anzahl der enthaltenen Trackpunkte läßt sich auch begrenzen. Wird sich nach meiner ersten Abschätzung aber eher an Leistungsfähigkeit der Browser orientieren als an der Leistungsfähigkeit von Server/Parser.

Was mir nicht so klar is:
Ist es sinnvoll oder nötig, bei der ganzen Sache mit stripslashes() zu arbeiten und wo und wie muß ich das dann einsetzen? Ist ja ne String-Funktion und keine Dateifunktion, denke ich.
Und bei mir wird doch mit einer Datei und nicht mit String gearbeitet, oder???

Viele Grüße
peter

[dedlfix]

echo $begrüßung;

if($_FILES['uploaddatei']['error']=='UPLOAD_ERR_OK')

UPLOAD_ERR_OK ist eine Konstante. Wenn du ihren Namen in '' setzt, wird der als String interpretiert. Du vergleichst hier also einen Zahlenwert in dem error-Element mit einem String. Das kommt sicher nur zufällig das gewünschte Ergebnis zustande, oder auch nicht. Hast du mal versucht, einen der anderen Fehler-Werte zu provozieren? Beispielsweise lässt sich ganz einfach UPLOAD_ERR_NO_FILE erzeugen, was in der Praxis sicher auch gelegentlich vorkommen wird.

$DATEI=$_FILES['uploaddatei']['tmp_name'];

Dieses Umkopieren des Wertes bringt dir außer einer zusätzlichen Programmzeile nichts weiter ein. Verwende doch gleich $_FILES['uploaddatei']['tmp_name'] an der einen Stelle, an der du nun $DATEI stehen hast.

$parser=xml_parser_create();

Was ist, wenn der Parser nicht erfolgreich kreiert werden konnte?

array_push($tracksegment,$trackpoint);

Auf array_push() sollte man zugunsten der Schreibweise $array[] = $wert; verzichten. Diese Funktion hat erst dann einen Nutzen, wenn man mehr als ein Element gleichzeitig anhängen will.

$gpxdatei=fopen($DATEI,'r');

Auch hier ist wieder keine Fehlerbehandlung zu sehen. Es ist zwar zu erwarten, dass bei einem erfolgreichen Upload die Datei auch vorhanden ist, doch nicht umsonst riecht es manchmal vor Apotheken etwas streng nach Reittierverdauungszwischenprodukten.

xml_parse($parser, $data, feof($gpxdatei));

xml_parse() beschwert sich regelmäßig bei ungültigem XML. Ignorier das nicht, wenn du dir Folgefehler wegen diese Vogel-Strauß-Taktik ersparen möchtest.

Ist es sinnvoll oder nötig, bei der ganzen Sache mit stripslashes() zu arbeiten und wo und wie muß ich das dann einsetzen?

stripslashes() benötigt man, wenn man anderweitig das Feature Magic Quotes nicht deaktiviert bekommt. In deinem Fall dürfte es nur dann zuschlagen, wenn irgendjemand entgegen der Default-Einstellung magic_quotes_runtime aktiviert hat.

echo "$verabschiedung $name";

[peter]

Hi dedlfix,

vielen Dank für deine zahlreichen Anregungen, werde das umfassend beherzigen.

if($_FILES['uploaddatei']['error']=='UPLOAD_ERR_OK')

UPLOAD_ERR_OK ist eine Konstante. Wenn du ihren Namen in '' setzt, wird der als String interpretiert. Du vergleichst hier also einen Zahlenwert in dem error-Element mit einem String. Das kommt sicher nur zufällig das gewünschte Ergebnis zustande, oder auch nicht. Hast du mal versucht, einen der anderen Fehler-Werte zu provozieren? Beispielsweise lässt sich ganz einfach UPLOAD_ERR_NO_FILE erzeugen, was in der Praxis sicher auch gelegentlich vorkommen wird.

Hab mir das mal angeschaut, die entsprechenden Konstanten haben ja auch Werte. Dann müßte ich doch so überprüfen können:

  
if($_FILES['uploaddatei']['error']==0)  
{  
//alles OK  
}  
  
if($_FILES['uploaddatei']['error']==4)  
{  
//keine Datei vorhanden  
}  

$DATEI=$_FILES['uploaddatei']['tmp_name'];

Dieses Umkopieren des Wertes bringt dir außer einer zusätzlichen Programmzeile nichts weiter ein. Verwende doch gleich $_FILES['uploaddatei']['tmp_name'] an der einen Stelle, an der du nun $DATEI stehen hast.

Zum Entwickeln mach ich das ganz gern so. Beschäftige mich erstmals mit dem Thema und ganz so leicht geht mir das auch nicht von der Hand. Da steckt schon ne Menge Arbeit und vor allem auch TRY and ERROR drin. Da isses wesentlich bequemer, $DATEI zur Verfügung zu haben und beim Programmieren nicht jedesmal $_FILES['uploaddatei']['tmp_name'] schreiben zu müssen. Ist ja auch mögliche Fehlerquelle (Flüchtigkeits-/Schreibfehler).

$parser=xml_parser_create();

Was ist, wenn der Parser nicht erfolgreich kreiert werden konnte?

$gpxdatei=fopen($DATEI,'r');

Auch hier ist wieder keine Fehlerbehandlung zu sehen.

xml_parse($parser, $data, feof($gpxdatei));

xml_parse() beschwert sich regelmäßig bei ungültigem XML.

Komme also vom Prinzip zu folgendem Ergebnis:

  
if($_FILES['uploaddatei']['error']==0)  
    {  
    //OK, weitermachen......  
    $parser=xml_parser_create();  
  
    if($parser)  
        {  
        //OK, weitermachen...  
        $gpxdatei=fopen($DATEI,'r');  
  
        if($gpxdatei)  
            {  
            //OK, weitermachen...  
             while ($data = fread($gpxdatei, 4096))  
                {  
                $parseOK=xml_parse($parser, $data, feof($gpxdatei));  
  
                if(!$parseOK)  
                    {  
                    //Fehler beim Parsen >ABBRUCH  
                    }  
                }  
            }  
        else  
            {  
            //Öffnen der Datei danebengegangen >ABBRUCH  
            }  
        }  
    else  
        {  
        //Parsererzeugung danebengegangen >ABBRUCH  
        }  
else  
    {  
    //irgendwas beim Dateiupload danebengegangen >ABBRUCH  
    //läßt sich mit ErrorCodes näher spezifizieren  
    }  
  

Eventuell wäre noch das Lesen der Datei ($data = fread($gpxdatei, 4096);) auf Erfolg zu überprüfen.

Vielen Dank nochmal für die vielen Hinwese und nützlichen Links.
Gruß
peter

[dedlfix]

echo $begrüßung;

if($_FILES['uploaddatei']['error']=='UPLOAD_ERR_OK')
Hab mir das mal angeschaut, die entsprechenden Konstanten haben ja auch Werte. Dann müßte ich doch so überprüfen können:
if($_FILES['uploaddatei']['error']==0)
if($_FILES['uploaddatei']['error']==4)

Ja, das geht prinzipiell so. Aber: wenn du den Konstanten-Namen einfach ohne die Anführungszeichen notierst, wird der Wert der Konstante beim Parsen/Kompilieren an ihrer Stelle eingesetzt. Es ist sprechender, wenn man die Konstanten verwendet, aus deren Namen schon ihre Bedeutung herauslesen kann, als nur die Zahl.

if ($_FILES['uploaddatei']['error'] == UPLOAD_ERR_OK)

$DATEI=$_FILES['uploaddatei']['tmp_name'];
Zum Entwickeln mach ich das ganz gern so. [...] Da isses wesentlich bequemer, $DATEI zur Verfügung zu haben und beim Programmieren nicht jedesmal $_FILES['uploaddatei']['tmp_name'] schreiben zu müssen. Ist ja auch mögliche Fehlerquelle (Flüchtigkeits-/Schreibfehler).

Ja, diese "Ausrede" hört man öfter von Anfängern. Es ist aber eigentlich so, dass man zwar denkt, sich Tipparbeit ersparen zu können, was in deinem Fall ja nun gerade nicht auftritt, stattdessen bringt man unnötige Komplexität durch noch mehr Variablen ins Spiel. Man kann aus dem mit $_FILES beginnenden Namen sofort deren Herkunft erkennen, bei $DATEI ist das nicht mehr der Fall. Je öfter du diese Tipparbeitserleichterung einsetzt, desto mehr musst du dir beim Nachvollziehen des Scripts Gedanken machen. Auch du wirst nach und nach Einzelheiten deines Script vergessen und stehst auf einmal davor, wie vor einem fremden Script und musst dich in seine Arbeitsweise einarbeiten. Da ist es dann von Vorteil, weniger komplex programmiert zu haben. Auch dass man die damaligen Gedankengänge, die zu einer bestimmten Lösung geführt haben, per Kommentar festgehalten hat erweist sich dann von Vorteil, aber das ist eine andere Baustelle.

$parser=xml_parser_create();
    if($parser)

Kann man so lassen, ist völlig o.k. Da du aber gern Tipparbeit sparst :-), hier ein Weg, wie man es auch notieren kann:

if ($parser = xml_parser_create())

Da das Ergebnis einer Zuweisung immer der Wert der Zuweisung selbst ist (sprich: bei if (a = b + c) ist das was das if zu sehen bekommt der gleiche Wert, der in a zu liegen kommt), kann man Zuweisung und Bedingung in der einen Zeile zusammenfassen. Man muss nun aber darauf achten, dass da statt des ==, das man hauptsächlich in if-Konstrukten findet, absichtlich ein = steht. Das sollte aber kein Problem darstellen, denn diese Notation verwendet man ja auch in while ($var = hol_funktion()).

echo "$verabschiedung $name";

[peter]

Hallo dedlfix,

vielen Dank.
Möchte hier jedoch noch mal auf meine eigentliche Kernfrage zurückkommen (ist in meiner Fragestellung vielleicht nicht so deutlich zu erkennen gewesen):

-Muß ich bei meinem Vorhaben in vorgestellter Ausführung Befürchtungen haben, daß irgendwelcher schädlicher Code hochgeladen/eingeschleußt/ausgeführt werden /Schaden anrichten kann?

-Welche zusätzlichen Sicherheitsmaßnahmen sind evtl. ratsam?

Gruß
peter