0


Edit 2 : I notices user can upload unlimited files and can take all disk space, how to prevent that?
Edit: since no one answered this question, is there a source I could read to get my answer???

I have a contact form. There are three inputs. I used a jQuery plugin for uploading files. This plugin adds another form element and uploads files by ajax.
I'm kind of beginner but this code is for a customer and a real job so I want to make sure it's safe!

in my view:

<form action="" method="post" enctype="multipart/form-data" >
<input type="text" name="name"  />
<input type="number" name="phone" />
<textarea name="enquiry" rows="10" ></textarea>
<div id="upload-div">
<div id="extraupload">Upload</div>
<input type="hidden" name="count" value="0" id="count"/>
<input type="submit" />

$(document).ready(function()
{
   var uploadObj = $("#extraupload").uploadFile({
      url:"/uplod_url",
      fileName:"file",
      onSuccess:function(files,data,xhr,pd)
      {
        data =  jQuery.parseJSON(data);
        if(data.status == 'success') {
          var count = $('#count').val() * 1 + 1;
          for(var i=0; i<data.files.length; i++) {
            $('<input type="hidden" name="file_'+count+'" value="'+data.files[i]+'">').insertBefore('#extraupload');
            $('#count').val(count);
            count++;
          }
        }
      },
    }); 
});
</script>

each successful upload,will add one to input count value and will append an hidden input with the value of uploaded file name.

In php I check for file type and change file name:

upload_url.php:

if ($_FILES['file']['type']=='image/jpeg' || $_FILES['file']['type']=='image/pjpeg') { 
    $ext = '.jpg';
}
elseif ($_FILES['file']['type']=='image/png') { 
    $ext = '.png'; 
}
elseif ($_FILES['file']['type']=='application/pdf') { 
    $ext = '.pdf'; 
}
else {
    echo json_encode('Only images and pdf files are allowed!');
    die();
}
$fileName = md5(uniqid());
$fileName = $fileName.$ext;
move_uploaded_file($_FILES["file"]["tmp_name"], 'image/tmp'.$fileName); 
$result = array('status'=> 'success','files' => $fileName);
echo json_encode($result);

After changing the file's name to a unique hash, I save that in a tmp folder.

then when the main form is submitted this is what happens:

//validation method: if that file exists in tmp folder  
if(isset($this->request->post['count']) && is_numeric($this->request->post['count'])) {
    for($i=1; $i<=$this->request->post['count']; $i++ ) {
        if(isset($this->request->post['file_'.$i])){
            if(!file_exists('image/tmp/'.$this->request->post['file_'.$i])){
                //throw error
            }
        } else{
            //throw error
        }               
    }
}
// hidden input count can only be integer
if(isset($this->request->post['count']) && !is_numeric($this->request->post['count'])) {
    //throw error
}

and then mailing the file and saving file name in database(I did not include database part because I'm kind of sure it's ok)

//by every submition delete files in tmp folder older than 1 day
$oldFiles = glob($tmp_dir."*");
$now   = time();

foreach ($oldFiles as $oldFile) {
    if (is_file($oldFile)) {
        if ($now - filemtime($oldFile) >= 60 * 60 * 24) { 
            unlink($oldFile);
        }
    }
}

$mail = new Mail();
//Mail Setting and details deleted

//if there's any file uploaded
if($this->request->post['count'] != 0) {
    //unique directory for every form submition
    $dir_path = 'image/submitted/'.uniqid();
    mkdir($dir_path, 0764, true);               

    //for all hidden inputs move file from tmp folder to $dir_path
    for ($i=1; $i <= $this->request->post['count']; $i++) {
        $file = $this->request->post['file_'.$i];
        rename('image/tmp'.$file, $dir_path.'/'.$file);
        $mail->AddAttachment($dir_path.'/'.$file);
    }
}
$mail->send();

now my question is: Is it safe this way? especially when I append hidden inputs with file's name and get the number of uploaded files from hidden input count??
This code already works, but I think this might be a security issue.
Thanks a lot for your patience and sorry for my poor english!
ps: I use opencart

Niloofar
  • 117
  • 1
  • 1
  • 13

1 Answers1

0

There is the general misconception that in AJAX applications are more secure because it is thought that a user cannot access the server-side script without the rendered user interface (the AJAX based webpage). XML HTTP Request based web applications obscure server-side scripts, and this obscurity gives website developers and owners a false sense of security – obscurity is not security. Since XML HTTP requests function by using the same protocol as all else on the web (HTTP), technically speaking, AJAX-based web applications are vulnerable to the same hacking methodologies as ‘normal’ applications.

  • thanks for your answer, so what can I do to make it secure? as I add in edit a user can upload unlimited files! – Niloofar May 20 '17 at 08:38
  • I suggest you find the clue [click here](http://stackoverflow.com/questions/256172/what-is-the-most-secure-method-for-uploading-a-file) and hope this one help you [click here](http://stackoverflow.com/questions/4950331/secure-php-file-upload-script) – Jaykumar Gondaliya May 20 '17 at 09:08