Bilder - Teil 2

[djfabi]

Hallo Leute,
es geht diesmal auch um Bilder aber diesesmal um den Upload, wie in meinem letzten Thread von schon jemandem angesprochen, ist das eine große Sicherheitslücke..

Deshalb wollte ich euch mal fragen, ob ihr mal über meins drübergucken könnt, und mit Tipps zur Sicherheit geben könntet :)
Hier der HTML:

<form method="post" action="upload.php" enctype="multipart/form-data">
<input type="file" name="image" /> <br/>
<input type="submit" name="submit" value="Upload" /> 
</form>

Noch nichts besonderes, hier die upload.php:

<?php

session_start();      
include('connect.php');

if(getimagesize($_FILES['image']['tmp_name']) == "" || !getimagesize($_FILES['image']['tmp_name'])){            header("Location: datei.php");            
}
       
if(isset($_POST['submit']) && $_POST['submit'] != "" && $_POST['submit'] == "Upload"){                      

// File Variables        
$name=  $_FILES['image']['name'];        
$temp=  $_FILES['image']['tmp_name'];       
 $type=  $_FILES['image']['type'];        
$size=  $_FILES['image']['size'];        
        
// Recognizing the extension        
switch( $type ){  
                              
// Image/Jpeg                
case 'image/jpeg':                        
$ext= '.jpg';                
break;                               

 // Image/png                
case 'image/png':                        
$ext= '.png';                
break;                                

// Image/gif                
case 'image/gif':                        
$ext= '.gif';                
break;                        
}                


$infos = @getimagesize($_FILES['image']['tmp_name']);                
$breite = $infos[0];                
$hohe = $infos[1];                
$dateityp = $infos[2];      
 
             
$zulaessige_formate = array(1, 2, 3);
$bildinformationen = @getimagesize($_FILES['image']['tmp_name']);
 
$fehler = true;            

if ($bildinformationen != false){  
if(in_array($bildinformationen[2], $zulaessige_formate))  {    

// echo "Das ist ein gueltiges Bildformat<br/>";                            
 // Upload Variables        
$path= 'ordner/bilder/'$irgenteinevariable_zufalsname_fur_bild.''.$ext;                

// Check for the Image post.       
 if( $_POST ){                        
    // Got into the POST check.                        
        if( $_FILES ){                                        
            // Got into the FILES check.                        
                if(move_uploaded_file( $temp, $path )){                                                        

$user_bild_pfad = str_replace("1internes/bilder/pb/", "", $path);   
                                                                                 
mysql_query("INSERT aksndkjasd (feld) VALUES ('$user_bild_pfad')"); 
 echo "<span>Das Bild <strong>".$name."</strong> wurde erfolgreich hochgeladen!</span><br/>";                                                                                                                                                                    

}                                                                                        
}                
}
                            
$fehler = false;  
}
}
 
if($fehler){
                            
echo "<span>Das Bildformat ist ungueltig. Erlaubt sind nur 'GIF', 'JPG' und 'PNG' Dateien.</span><br/>";                            
  exit;
}                
}else{                      
header("Location: datei.php");                                 
}                                

?>

Ist das Sicher?

 

[bdt600]

Wenn man den Quellcode vernünftig lesen könnte, könnte man vielleicht auch was dazu sagen.

 

[djfabi]

Wenn man den Quellcode vernünftig lesen könnte, könnte man vielleicht auch was dazu sagen.

Ich habe ihn gerade etwas eingerückt, hoffe es ist nun lesbar :)

 

[bdt600]

Mal abgesehen von Sicherheit, das ist Chaos pur!

Am Anfang

if(getimagesize($_FILES['image']['tmp_name']) == "" || !getimagesize($_FILES['image']['tmp_name']))

Dann

if(isset($_POST['submit']) && $_POST['submit'] != "" && $_POST['submit'] == "Upload")

Dann

 if( $_POST )

Dann

if( $_FILES )

und mitten drin 2x

@getimagesize($_FILES['image']['tmp_name']);

Ich würde mal vorschlagen, bring' da erst mal Ordnung rein.

Beispiel:

<?php
   session_start();

   include('connect.php');

   if(isset($_POST['submit']))
   {
       if ($_FILES['image']['error'] == UPLOAD_ERR_OK)
       {
           $infos = @getimagesize($_FILES['image']['tmp_name']);

           if ($infos !== false)
           {
               // Hier geht es erst los mit der Verarbeitung
           }
           else
             echo "Keine Bilddatei";
       }
       else
         echo "Fehler beim Upload, Error-Code: " . $_FILES['image']['error'];
   }
   else
     echo "Keine Daten";
?>

 

[djfabi]

Ok, aber mal eine Frage, du gibst da Keine Bilddatei! aus, woher weißt du das?

Und wie soll es dann weiterverarbeitet werden? Also mit dem mysql_query?
Und ist es dann 100%ig sicher?

Sorry für die blöden Fragen

 

[thuemmy]

Weil die Funktion getimagesize "false" zurückliefert wenn keine Bilddatei als Parameter angegeben wurde.

 

[djfabi]

Ok, danke, noch ein paar Fragen:
-Und wie soll es dann weiterverarbeitet werden? Also mit dem mysql_query?
-Und ist es dann 100%ig sicher?

 

[Cheffchen]

Hallo,

denk an "image/pjpeg", das gibt es auch noch wenn an typen filterst.

Das mit mysql query hattest doch schon richtig oben, nicht den dateinamen übernehmen sondern ein eigenen einteutigen geben.

Cheffchen

 

[djfabi]

ok, das mit nem eigenen Dateinamen hatte ich schon, habe es nur editiert ;)
Wirklich header('Content-Type: image/pjpeg');
?
Oder meintest du:
header('Content-Type: image/jpeg');
Nur ein Problem:
Wenn da nur image/jpeg steht, ist ja nix mit .gif und .png oder?

 

[bdt600]

 <?php
   session_start();

   include('connect.php');

   if(isset($_POST['submit']))
   {
       if ($_FILES['image']['error'] == UPLOAD_ERR_OK)
       {
           $infos = @getimagesize($_FILES['image']['tmp_name']);

           if ($infos !== false)
           {
               // Hier geht es erst los mit der Verarbeitung
               if ($_FILES['image']['mime'] == 'image/gif' ||
                   $_FILES['image']['mime'] == 'image/jpeg' ||
                   $_FILES['image']['mime'] == 'image/png')
               {
                   $filename = uniqid();
                   
                   switch($_FILES['image']['mime'])
                   {
                       case 'image/gif'  : $filename .= '.gif'; break;
                       case 'image/jpeg' : $filename .= '.jpg'; break;
                       case 'image/png'  : $filename .= '.png'; break;
                   }
                   
                   $path = $_SERVER['DOCUMENT_ROOT'] . '/bilder/';
                   
                   if (move_uploaded_file($_FILES['image']['tmp_name'], $path . $filename))
                   {
                      $query = "INSERT 
                                   into
                                     `tabelle`
                                   set
                                     `filename`='" . $path . $filename . "'";
                                     
                      mysql_query($query)
                          or die("MySQL-Error: " . mysql_error());
                          
                     echo "Datei erfolgreich übertragen und in Datenbank eingetragen";
                   }
                   else
                      echo "Fehler beim Kopieren der Upload-Datei";  
               }
               else
                  echo "Unzulässiger Dateityp";
           }
           else
             echo "Keine Bilddatei";
       }
       else
         echo "Fehler beim Upload, Error-Code: " . $_FILES['image']['error'];
   }
   else
     echo "Keine Daten";
?>

 

[djfabi]

Vielen Dank :))
Sieht sehr sicher aus, ist das auch so?
Dumme Frage

tausend danke! :)