PHPMailer file upload is this correct

Hi I have been struggling with working out the correct way to structure file uploads. With a lot of help I have managed to get it working but want to make sure that what I have come up with is correct and secure. Here is the full code. One thing that occurred to me is if I am moving the files to a temp directory and then compressing them to a desired directory do the files still exist in the temp directory? Do I need to delete those?

Here is the full code:

// Import PHPMailer classes into the global namespace
// These must be at the top of your script, not inside a function

use PHPMailerPHPMailerPHPMailer;
use PHPMailerPHPMailerSMTP;
use PHPMailerPHPMailerException;

require 'autoloader.php';

$mail = new PHPMailer(true);



$clean_email = filter_var($_POST['appEmail'],FILTER_SANITIZE_EMAIL);


$files = "";
$target_dir = $_SERVER['DOCUMENT_ROOT'] . "/XXXXXXX/uploads/";

$dateKey = date( 'd-m-Y--H-i-s' );

function clean($string) {
    $string = str_replace(' ', '-', $string); // Replaces all spaces with hyphens.

    return preg_replace('/[^A-Za-z0-9-]/', '', $string); // Removes special chars.
}

function compressImage($source, $destination, $quality) {

    $info = getimagesize($source);

    if ($info['mime'] == 'image/jpeg') {
        $image = imagecreatefromjpeg($source);
    } elseif ($info['mime'] == 'image/gif') {
        $image = imagecreatefromgif($source);
    } elseif ($info['mime'] == 'image/png') {
        $image = imagecreatefrompng($source);
    }

    imagejpeg($image, $destination, $quality);

}

if(isset($_FILES)) {



    $uploadOk = 1;
    $fileString = '';

    $fileMessage = 'FILEERROR(';

    $files = $_FILES;


    foreach ( $files as $key => $file ) {

        $uploadfile = tempnam(sys_get_temp_dir(), hash('sha256', $file['name']));

        if (move_uploaded_file($file['tmp_name'], $uploadfile)) {



//get the file extension
        $imageFileExt = strtolower( pathinfo( $file["name"], PATHINFO_EXTENSION ) );


// get the file type
        $target_file = $target_dir . basename($file["name"]);

        $imageFileType = strtolower(pathinfo($target_file,PATHINFO_EXTENSION));


// get file size
        $check = getimagesize($uploadfile);
        if($check === false) {
            $fileMessage .=  $key."=noimage,";
            $uploadOk = 0;
            $msg.="noimage";
        }

// Allow certain file formats

        else if($imageFileType !== "jpg" && $imageFileType !== "png" && $imageFileType !== "jpeg"
                && $imageFileType !== "gif" ) {
            $fileMessage .=  $key."=wrongfile,";
            $uploadOk = 0;
            $msg.="wrongeimage";
        }

// Check if file already exists
        else if (file_exists($target_file)) {
            $fileMessage .=  $key."=fileexists,";
            $uploadOk = 0;
            $msg.="fileexists";
        }


// Check file size
        else if ($file["size"] > 20000000) { //20mb
            $fileMessage .=  $key."=toobig,";
            $uploadOk = 0;

            $msg.="toobig";

        } else {

            //create a new name for the file
//the persons name plus file field plus date plus time plus file extension
//such as :joe_bloggs_bank_statement_1_9_10_21_10_55.jpg
//and joe_bloggs_pay_slip_1_9_10_21_10_55.jpg


            $fileName = clean($_POST['appName']). "_" . $key . "_" . $dateKey . "." . $imageFileType;
            compressImage($uploadfile, $target_dir . basename( $fileName ), 60);



            $msg .= "comp";
        }



// creates a set of links to the uploaded files on the server
        // to be placed in the body of the email (the files themselves do not get attached in the email
        $fileString .= strtoupper($key).": <a href='XXXXXXXX/uploads/".$file['name']."'>".$file['name']."</a><br>";

        $msg.="upload success";

    } else{
            $msg.="cant upload files";
        }
    }
    $fileMessage .= ')';


}






try {
    //Server settings
    //$mail->SMTPDebug = SMTP::DEBUG_SERVER;                      // Enable verbose debug output
    $mail->isSMTP();                                            // Send using SMTP
    $mail->Host       = 'mail.XXXXXXXXX.com';                    // Set the SMTP server to send through
    $mail->SMTPAuth   = true;                                   // Enable SMTP authentication
    $mail->Username   = 'XXX@XXXXXXX.com';                     // SMTP username
    $mail->Password   = 'XXXXXXXXXX';                               // SMTP password
    $mail->SMTPSecure = PHPMailer::ENCRYPTION_STARTTLS;         // Enable TLS encryption; `PHPMailer::ENCRYPTION_SMTPS` encouraged
    $mail->Port       = 587;                                    // TCP port to connect to, use 465 for `PHPMailer::ENCRYPTION_SMTPS` above

    //Recipients
    $mail->setFrom( 'XXX@XXXXXXXXX.com', 'Mailer' );
    $mail->addAddress( 'XXXXX@XXXXXXXXX.com', 'Recipient' );     // Add a recipient

    $mail->addReplyTo( $_POST['appEmail'], 'Information' );


    // Content
    $mail->isHTML( true );                                  // Set email format to HTML
    $mail->Subject = 'Here is the subject';
    $mail->Body    = 'This is the HTML message body <b>in bold!</b>';
    $mail->AltBody = 'This is the body in plain text for non-HTML mail clients';

    $mail->send();


    echo 'Message sent successfully'.$msg;
} catch (Exception $e) {
    echo "Message could not be sent. Mailer Error: {$mail->ErrorInfo}";
}

Answer

move_uploaded_file() does what it says: it moves the file, so it is no longer in the original temp directory, but wherever you moved it to.

Your series of if/elseif checks will only ever match a single problem – it’s nicer for your users if you detect all the problems in one go rather than just one at a time, so most of those elseifs should just be ifs.

This looks wrong:

if (file_exists($target_file)) {

$target_file contains the original user-supplied filename, not the moved filename ($uploadfile), so I would not expect a file with that name to exist.

This looks like a typo: $msg.="wrongeimage";

Advertisement

Answer