for-Schleife
Manu3l
- php
0 Rolf B0 Manu3l0 Rolf B
0 Felix Riesterer0 Robert B.0 Rolf B
0 TS- php
- sicherheit
Hallo,
ich habe eine Frage zur for-Schleife.
Ich habe mir für nur freigegebene Benutzer eine Upload-Seite gescriptet. Es funktioniert auch soweit alles, nur meine for-Schleife macht irgendwie keine richtige Schleife...
Am Anfang habe ich mehrere Abfragen, (ob ein neues Album angelegt wurde, Dateien ausgewählt wurden usw.) dann beginnt das eigentliche Script zum Hochladen für mehrere Dateien.
// Anzahl Dateien zählen
$countfiles = count($_FILES['file']['name']);
// Loop aller Dateien durchführen
for($i=0;$i<$countfiles;$i++)
{
$filename = $_FILES['file']['name'][$i];
$zugelassenedateitypen = array("image/png", "image/jpeg", "image/gif", "application/x-zip-compressed"/*, "application/octet-stream"*/);
if ( ! in_array( $_FILES['file']['type'][0] , $zugelassenedateitypen ))
{
echo "<p>Jedoch ist der Dateityp NICHT zugelassen! Aus diesem Grund konnte nichts hochgeladen werden!</p>";
}
else
{
// Test ob Dateiname in Ordnung
$filename = dateiname_bereinigen($filename);
if ( $_FILES['file']['name'][0] <> '' )
{
// Datei von Temp-Ordner in angegebenen Ordner verschieben
move_uploaded_file($_FILES['file']['tmp_name'][$i],'uploads/'.$ordner.'/'.$filename);
echo "<p>Hochladen war erfolgreich: ";
echo '<a href="uploads/' .$ordner.'/'.$filename .'">';
echo 'uploads/'.$ordner.'/'.$filename;
echo '</a>';
}
else
{
echo "<p>Dateiname ist nicht zulässig</p>";
}
}
}
"dateiname_bereinigen" ist meine Funktion, die festlegt, ob Umlaute, Leerzeichen usw. in der Datei sind und ändert diese gegebenenfalls auch ab. Dies funktioniert auch mit zwei oder mehr Dateien.
"$zugelassenedateitypen" prüft, ob es eine nicht von mir festgelegte Datei ist. (z.B. .PHP, .doc usw.) Leider prüft er dies nur bei der ersten Datei, was auch funktioniert, aber sobald ich zwei oder mehr Dateien hochladen möchte, führt er für die kommenden Dateien diese Prüfung nicht mehr aus und läd z.B. eine PHP-Datei auf den Server.
Meine for-Schleife sollte aber doch jedes mal greifen und die Dateiprüfung durchführen. Oder habe ich da irgendeinen Denkfehler?
Danke!
Manuel
Hallo Manu3l,
habe ich da irgendeinen Denkfehler?
Nein, sondern eine unvollständige Code-Änderung.
Du hat beim Erweitern von 1 auf N Dateien nicht überall [0] durch [$i] ersetzt.
Weitere Hinweise:
$zugelassenedateitypen ist in jedem Schleifendurchlauf gleich. Die Initialisierung dieser Variablen sollte vor der FOR-Schleife erfolgen
if ( $_FILES['file']['name'][0] <> '' )
- Wieso greifst Du hier wieder auf das $FILES Array zu? Warum nicht auf $filename?
dateiname_bereinigen - Leerzeichen und Umlaute sind gültige Zeichen in Dateinamen. Warum entfernen? Gefährlicher sind Zeichen wie /
, \
oder :
. Aber das mag sein wie es will, denn:
den vom User gelieferten Filename solltest Du auf KEINEN Fall in dein eigenes Filesystem lassen. Allein schon deshalb nicht, damit der User X nicht Dateien von User Y überschreiben kann. Lege die Datei unter einem selbstvergebenen Namen ab und speichere Dir irgendwo (in einer Datenbank zum Beispiel), dass User X die Datei Y hochgeladen hat, sie vom Mime-Typ foo/bar ist und sie im Upload-Bereich unter dem Namen "mklmtciohremctgk" liegt.
Ich hoffe übrigens, dass Du einen Virenscanner über das Upload-Gut laufen lässt. Wenn Du ZIP-Archive annimmst, kann darin alles mögliche Ungeziefer stecken, und "application/octet-stream" ist noch schlimmer.
Rolf
Hallo Rolf,
zu 1) Dachte, das Array muss innerhalb der Schleife stehen. Hab es geändert
zu 2) Da greife ich nicht auf $filename zu, da ich hier den [Typ] benötige und bei $filename ist der [name] hinterlegt
zu 3) Meinte damit "ä", "ü" usw. aber Zeichen wie "/" und "" nehme ich auch raus.
Damit die Datei nicht überschrieben wird, hänge ich an jeden Dateinamen Datum und Uhrzeit mit Sekunden. Somit umgehe ich das Problem, ohne dass ich einen kompletten neuen Namen vergeben muss.
Einen Virenscanner lasse ich tatsächlich nicht drüberlaufen. Muss ich mich mal schlau machen.
"application/octet-stream" habe ich extra auskommentiert. Aber ich lasse es drin, falls doch mal eine .rar Datei hochgeladen werden muss. Leider funktioniert .rar nicht mit "application/x-rar-compressed". Finde dazu leider auch nicht wirklich was im Internet, bzw. ähnliche Probleme wie ich sie habe.
Ich habe die zwei [0] durch [$i] ersetzt und es funktioniert wunderbar. Hätte ich auch selbst drauf kommen können. Hab mich aber nur mit der Schleife beschäftigt...
Ich danke dir!
Manuel
Hallo Manu3l,
du hast aber gesehen, dass ich bei Punkt 2 nicht den Typzugriff, sondern den Namenszugriff angesprochen hatte?
Rolf
Lieber Rolf,
Ich hoffe übrigens, dass Du einen Virenscanner über das Upload-Gut laufen lässt.
ich hoffe, dass er das nicht tut. Die Dateien sollten via HTTP-Request nicht erreichbar sein, so dass auch keine Shell-Scripte hochgeladen und ausgeführt werden können. Aber ein Virenscanner auf dem Server halte ich für Unsinn.
Wenn Du ZIP-Archive annimmst, kann darin alles mögliche Ungeziefer stecken, und "application/octet-stream" ist noch schlimmer.
Das ist aber das Problem des Clients und nicht des Servers. Wer sich das Ungeziefer herunterlädt und ein dafür anfälliges System (Windoof) hat, sollte eben damit leben können, dass er oder sie eventuell ein verwanztes System hat.
Im Übrigen schließe ich mich Fefes Meinung zu Schlangenöl an.
Liebe Grüße
Felix Riesterer
Hallo Rolf,
Ich hoffe übrigens, dass Du einen Virenscanner über das Upload-Gut laufen lässt. Wenn Du ZIP-Archive annimmst, kann darin alles mögliche Ungeziefer stecken, und "application/octet-stream" ist noch schlimmer.
Und wenn das ZIP-Archiv bösartig ist, hat dein Virenscanner Spaß 😉
application/octet-stream
ist übrigens der Default-MIME-Typ etlicher Browser beim Upload, wenn nichts Anderes passt. Warum soll das gefährlicher als andere MIME-Types sein?
Viele Grüße
Robert
Hallo Robert,
oh, stimmt. Wenn ein Bild auch als octet-stream kommt, ist die Argumentation Quatsch.
Rolf
Hello,
ganz schlechte Idee, dem vom Client gemeldeten MIME-Type zu vertrauen.
Lies dir bitte did umfangreichen Gedanken im Fileupload-Artikel durch.
Glück Auf
Tom vom Berg