Image Upload sicher?

[one6666]

Hallo,

habe schon ewigkeiten nix mehr mit PHP gemacht,
nun brauchte ich aber für meinen Image-Resizer(Javascrript) ein Bild Uploader,
bin mir gerade nicht sicher ob ich nicht vielleicht etwas übersehen haben,
kann mal Jemand schauen ob der so sicher ist ?

<?php
$upload_file = getimagesize($_FILES['upload_file']['tmp_name']);

switch($upload_file[2]){
	case 1: // GIF
	$upload_type = "gif";
	break;
	case 2: //JPG
	$upload_type = "jpg";
	break;
	case 3: //PNG
	$upload_type = "png";
	break;
}
	
$file_id     = time();              
$file_id    *= mt_rand(1,100);   

if($upload_file[2] != 0 && $upload_file[2] < 4){     // Only GIF, JPG and PNG
	if($_FILES['upload_file']['size'] < 10485760){   // Max. Size 10MB
	    if(move_uploaded_file($_FILES['upload_file']['tmp_name'], 'upload/' . $file_id . "." . $upload_type)){
			echo "Datei wurde erfolgreich hochgeladen der neue Datei Name heißt:" . $file_id . "." . $upload_type;
		}else{
			echo "Es ist ein Technischer Defekt aufgetretten bitte versuchen Sie es in 5 Minuten wieder";
		}
	}else{
		echo "Datei zu Groß Max. Size 10MB";
	}
}else{
	echo "Fehler:Unerlaube Datei"; 
}
?>

 

[Sven Mintel]

Moin,

ansich sieht das gut aus.

Du könntest noch $_FILES['upload_file']['error'] prüfen.

Ob es Sicherheitsprobleme gibt, kann man nicht pauschal sagen, das hängt zum grossen Teil auch davon ab, wie sicher die restlichen Skripte auf dem Server sind, z.B. gegen das unbefugte includen nicht dafür vorgesehener Dateien vom Server(so ein Bild kann auch gut PHP-Code enthalten)

Aber rein so den Upload betrachtet, würde ich sagen das ist OK

 

[one6666]

Hallo Sven,

das $_FILES['upload_file']['error'] war mir Gestern oder ist mir auch immer noch ein Rätsel,
für mich wäre dann ja nur:

UPLOAD_ERR_OK

Wert: 0; Es liegt kein Fehler vor, die Datei wurde erfolgreich hochgeladen.

interessant, das gleiche erreiche ich aber auch in dem ich die Funktion move_uploaded_file in eine If Anweisung lege,
auch die liefert nur true zurück wenn wirklich alles geklappt hat:

Im Erfolgsfall gibt diese Funktion true zurück. Sollte es sich bei der hochgeladenen
Datei um keine gültige Datei handeln oder konnte sie nicht verschoben werden,
so wird false zurückgegeben.

oder habe ich einen Denkfehler in meiner Logik ?

mfg.one6666

 

[Sven Mintel]

move_uploaded_file() liefert dir halt ein Feedback, ob die Datei verschoben werden konnte.

Für den User liesse sich über den Fehlercode eine genauere Fehlermeldung erstellen.

 

[one6666]

Hi Sven,

hatte erst garnicht verstanden wo der unterschied liegen sollte:-(,
bessere Fehlermeldungen wären mir auch ganz recht aber die Auswahl liefert jetzt nix was ich schon habe bis auf,

UPLOAD_ERR_PARTIAL

Wert: 3; Die Datei wurde nur teilweise hochgeladen.

Aber das wäre jetzt eine so große Ausnahme das ich da für mich keinen Sinn drin sehe

Hier der fertige Code:

<?php
$upload_file = getimagesize($_FILES['upload_file']['tmp_name']);

switch($upload_file[2]){
	case 1: // GIF
	$upload_type = "gif";
	break;
	case 2: //JPG
	$upload_type = "jpg";
	break;
	case 3: //PNG
	$upload_type = "png";
	break;
}
	

$file_id     = time();              
$file_id    *= mt_rand(1,100); 

$script_start = "<script type='text/javascript'>";
$script_end   = "</script>";

$error_list = array();
$error_list[] = "Es ist ein unerwarteter Fehler aufgetreten,<br /> bitte versuchen Sie es in 5 Minuten wieder.<br />Wir danken für Ihr Verständnis.";
$error_list[] = "Das von Ihnen gewählte Bild ist zu Groß!<br />Die maximale Bild Größe beträgt 10Megabyte(MB).<br />Wir danken für Ihr Verständnis.";
$error_list[] = "Das von Ihnen gewählte Bild hat ein unerlaubtes Datei Format!<br />Es sind nur Bilder im Format JPG, GIF und PNG erlaubt.<br />Wir danken für Ihr Verständnis.";

if($upload_file[2] != 0 && $upload_file[2] < 4){     
	if($_FILES['upload_file']['size'] < 10485760){   
	    if(move_uploaded_file($_FILES['upload_file']['tmp_name'], 'upload/' . $file_id . "." . $upload_type)){
			echo $script_start . "window.top.window.upload_stop(1, '". $file_id . "." . $upload_type ."');" . $script_end;
		}else{
			echo $script_start . "window.top.window.upload_stop(2, '" . $error_list[0] . "');" . $script_end;
		}
	}else{
		echo $script_start . "window.top.window.upload_stop(2, '" . $error_list[1] . "');" . $script_end;
	}
}else{
	echo $script_start . "window.top.window.upload_stop(2, '" . $error_list[2] . "');" . $script_end;
}
?>

 

[Sven Mintel]

Naja, mal angenommen, es wurde garnichts hochgeladen(ist ja nicht ungewöhnlich)...dann würde getimagesize() zumindest ein Warning produzieren...das würde nicht passieren, wenn du vorher $_FILES['upload_file']['error'] prüfen und ggf. darauf reagieren würdest.