-1

I'm trying to make it so that when the user uploads any document that isn't a csv file an echo error appears. However when I tested it I got the echo error on both the correct file type and an incorrect file type. Anyone know where I'm going wrong?

<?php
    ob_clean();session_start();

    if (isset($_GET['logout'])){
    session_destroy();  
    }

    if (!isset($_SESSION['loggedin']) || $_SESSION['loggedin'] == false) {
        header("Location: index.php");
    }

    if(isset($_FILES['UploadFileField'])){
    $allowed = array('csv');
    $UploadName = $_FILES['UploadFileField']['name'];
    $UploadTmp = $_FILES['UploadFileField']['tmp_name'];
    $UploadType = $_FILES['UploadFileField']['type'];
    $NewFileName = "project1file.txt";


    if(!$UploadTmp){
        echo '<font color="#FF0000" size="3"><p align="center"><b>No File Selected, Please Try Again.</b></p></font>';
    }else{
        move_uploaded_file($UploadTmp, "UPLOADS/$NewFileName");
        echo '<font color="#006600" size="3"><p align="center"><b>File Successfully Uploaded.</b></p></font>';

    }

    if(!in_array($UploadTmp,$allowed) ) {
    echo 'error';
}

}

?>
SpudMcKenzie
  • 39
  • 1
  • 7
  • 2
    `$_FILES['UploadFileField']['tmp_name'];` simply does not contain the file extension: it contains the path to the temporary file. Use `var_dump($UploadTmp)` to see what is in a variable. Solve this problem, by only looking at the extension of the file. You can do so by using `explode('.', $UploadName)` and take the last part of the array it produces. You can also try to use the mime type in `$UploadType`. Note, however that a user can upload files and manually change the extension. So it is not guaranteed that the content is actually CSV. – user228395 Feb 23 '16 at 12:48
  • 1
    use `pathinfo($UploadTmp , PATHINFO_EXTENSION);` this fetches extension – Thamilhan Feb 23 '16 at 12:52
  • Okay so I tried `$ext = pathinfo($UploadName, PATHINFO_EXTENSION);` and echo `$ext` instead but this time didn't echo anything? – SpudMcKenzie Feb 23 '16 at 13:04
  • Didn't see the previous comment, however I tried the same with `$UploadTmp` and got the same results – SpudMcKenzie Feb 23 '16 at 13:05
  • Check the mime type of the file dont trust the extention. I have seen sites that were hacked using images with php code in – Jacques Koekemoer Feb 23 '16 at 13:40

3 Answers3

0

Use the below code to find the extension of the uploaded file:

$type = $_FILES["UploadFileField"]["type"];

And then echo

if(!in_array($type,$allowed) ) {
    echo 'error';
}

Update 1:

$mimes = array('application/vnd.ms-excel','text/plain','text/csv','text/tsv');

if(in_array($_FILES['UploadFileField']['type'],$mimes)){
  // do something
} else {
  die("Sorry, mime type not allowed");
}

Update 2:

You can use this as well, actually $_FILES...['type'] is not safe to use.

$types = array('csv');

$ext = pathinfo($UploadName, PATHINFO_EXTENSION);


if(in_array($ext,$types)){
// do something
} else {
    die("Sorry, only CSV type allowed");
}
Fakhruddin Ujjainwala
  • 2,493
  • 17
  • 26
  • ...and check `$_FILES['UploadFileField']['type']` for containing "text/csv" – syck Feb 23 '16 at 13:03
  • This has done the trick thank you, how would I prevent it from uploading if it isn't a CSV very simpler to how I've done it with no file uploaded? – SpudMcKenzie Feb 23 '16 at 13:11
  • @MatthewHutchings U mean to say you just want to allow CSV files? – Fakhruddin Ujjainwala Feb 23 '16 at 13:13
  • This works however, I don't want the user to be able to upload other document types that excel provides? – SpudMcKenzie Feb 23 '16 at 13:38
  • @MatthewHutchings tell me which specific ones do you want to allow? – Fakhruddin Ujjainwala Feb 23 '16 at 13:46
  • Would changing `die` to `echo` make a difference as I know `die` will stop the script from running but I would rather it appear as an echo as it looks cleaner on my site. Also how would I put my `if(!$UploadTmp){echo '...'}` back in as I put it before the `if(in_array($ext,$types)` and when I tested it I got both echos appear for no file and wrong type? – SpudMcKenzie Feb 23 '16 at 13:49
  • @FakhruddinUjjainwala As to your reply, the only document I wanted to be able to upload was a CSV file however I believe your Update 2 provided this. Just need to crack the issue with no file being uploaded. Thank you for helping me. – SpudMcKenzie Feb 23 '16 at 13:58
  • I would be more than happy to if I had the reputation high enough to do so, could I be a pain and ask for assistance with the echo problem I'm having after incorporating this new method? – SpudMcKenzie Feb 23 '16 at 14:09
  • @MatthewHutchings no its ok. What kind of problem? – Fakhruddin Ujjainwala Feb 23 '16 at 14:12
  • @FakhruddinUjjainwala In the first post I had a section of code `if(!$UploadTmp){ echo '...' }` now I still want to be able to test for this before testing to see if the file is a CSV as I tried moving it above and playing around with it but all I could get was both echo's to appear rather than one for each error? – SpudMcKenzie Feb 23 '16 at 14:20
  • If I understand you correctly, you would either have to move `if(!in_array(` into the else branch of the first if, or have the condition of the second if check `$UploadTmp`, like `if($UploadTmp && !in_array(...`. – syck Feb 23 '16 at 14:38
  • @MatthewHutchings in short do you want to know if the file has been selected or its empty? – Fakhruddin Ujjainwala Feb 23 '16 at 14:38
  • To use die in a webscript is a rather bad idea, as it exits immediatly and therefore will probably deliver incomplete output. And @MatthewHutchings, there should be another test that rejects on illegal characters or improper formatting. – syck Feb 23 '16 at 14:41
  • Never mind I've cracked it, you were correct with the`if(in_array(` into the else branch of the first if. Thank you for your help. – SpudMcKenzie Feb 23 '16 at 14:43
  • @syck What type of thing are you suggesting? – SpudMcKenzie Feb 23 '16 at 14:44
0

Although csv does have an RFC and hence a mimetype, there are still a lot of devices which are not pre-configured with the appropriate mimetype (but note that CSV is actually a family of formats).

Both the mimetype and extension are assertions by the client about the content of the file and should not be trusted.

As to why your code is not doing what you expect....you are comparing the mimetype (which should be 'text/csv') with 'csv'. They are not the same.

As to where you are going wrong....

Your code contains no comments. You could have found the problem yourself by instrumenting the code to detail what was actually arriving at the server. You shouldn't be calling move_uploaded_file() before validating the file, and you should probably have a more robust method for validating the file.

Community
  • 1
  • 1
symcbean
  • 47,736
  • 6
  • 59
  • 94
0

Please add 3 variables in isset file condition.

1.$target_dir = "uploads/";
2.$target_file = $target_dir . basename($_FILES["UploadFileField"]["name"]);
3.$FileType = pathinfo($target_file,PATHINFO_EXTENSION); 

so condition is

   if (isset($_FILES['UploadFileField']))
    {
        $allowed = array('csv');
        $UploadName = $_FILES['UploadFileField']['name'];
        $UploadTmp = $_FILES['UploadFileField']['tmp_name'];
        $UploadType = $_FILES['UploadFileField']['type'];
        $NewFileName = "project1file.txt";
        $target_dir = "uploads/";
        $target_file = $target_dir . basename($_FILES["UploadFileField"]["name"]);

        $FileType = pathinfo($target_file, PATHINFO_EXTENSION);
    }

    if (!$FileType)
    {
        echo '<font color="#FF0000" size="3"><p align="center"><b>No File Selected, Please Try Again.</b></p></font>';
    }
    else
    {
        move_uploaded_file($UploadTmp, $target_file);
        echo '<font color="#006600" size="3"><p align="center"><b>File Successfully Uploaded.</b></p></font>';
    }

    if (!in_array($FileType, $allowed))
    {
        echo 'error';
    }  

Please check your folder name. It is now "upload".

Amit Chaudhari
  • 397
  • 1
  • 3