how to upload images in flash/php securely?

[foevah]

I've got a script that lets me upload anything but when I try to make it secure by adding code that only accepts a certain image file type and size, it doesn't work anymore!?

Heres a link to the working flash uploader

http://www.jamesgardner.lincoln.ac.uk/upload_flash8/upload.swf

all the images uploaded are stored here

http://www.jamesgardner.lincoln.ac.uk/upload_flash8/files/

 

This script below works:

<?php
//create the directory if doesn't exists (should have write permissons)
if(!is_dir("./files")) mkdir("./files", 0755); 
//move the uploaded file
move_uploaded_file($_FILES['Filedata']['tmp_name'], "./files/".$_FILES['Filedata']['name']);
chmod("./files/".$_FILES['Filedata']['name'], 0777);
?>

 

When I add the following line of code to the script above it doesnt work!?

 

<?php

if (($_FILES["uploaded_file"]["type"] == "image/jpeg") && ($_FILES["uploaded_file"]["size"] < 350000)) {

//create the directory if doesn't exists (should have write permissons)
if(!is_dir("./files")) mkdir("./files", 0755); 
//move the uploaded file
move_uploaded_file($_FILES['Filedata']['tmp_name'], "./files/".$_FILES['Filedata']['name']);
chmod("./files/".$_FILES['Filedata']['name'], 0777);
?>

 

What am I doing wrong? Please can someone help me!

[designationlocutus]

Do you have the closing brace on your 'if statement'?

[foevah]

do you mean add ; at the end? So it would look like this:

 

<?php
if (($_FILES["uploaded_file"]["type"] == "image/jpeg") && ($_FILES["uploaded_file"]["size"] < 350000)) ;

//create the directory if doesn't exists (should have write permissons)
if(!is_dir("./files")) mkdir("./files", 0755); 
//move the uploaded file
move_uploaded_file($_FILES['Filedata']['tmp_name'], "./files/".$_FILES['Filedata']['name']);
chmod("./files/".$_FILES['Filedata']['name'], 0777);
?>

 

I just tried that and it still accepts gifs

[arbol]

no not ;, you need a } to finish the entire if statement.

 

<?php

 

if (($_FILES["uploaded_file"]["type"] == "image/jpeg") && ($_FILES["uploaded_file"]["size"] < 350000)) { // thats the opening bracket

 

//create the directory if doesn't exists (should have write permissons)

if(!is_dir("./files")) mkdir("./files", 0755);

//move the uploaded file

move_uploaded_file($_FILES['Filedata']['tmp_name'], "./files/".$_FILES['Filedata']['name']);

chmod("./files/".$_FILES['Filedata']['name'], 0777);

 

} // thats what you were missing!

?>

[designationlocutus]

No, try the code below:

 

<?php
if (($_FILES["uploaded_file"]["type"] == "image/jpeg") && ($_FILES["uploaded_file"]["size"] < 350000))  {

//create the directory if doesn't exists (should have write permissons)
if(!is_dir("./files")) mkdir("./files", 0755) {
//move the uploaded file
move_uploaded_file($_FILES['Filedata']['tmp_name'], "./files/".$_FILES['Filedata']['name']);
chmod("./files/".$_FILES['Filedata']['name'], 0777);
}
}
?>

[foevah]

ah ha yes! thank you every much designactionlocutus and arbol! You are both life and hacker savers!

Bless.

[designationlocutus]

You're welcome and the best of luck with your project.

[foevah]

still doesn't work

 

heres the PHP. I've tried changing uploaded_file in the first if statement to Filedata but nothing works..

 

<?php
if (($_FILES["uploaded_file"]["type"] == "image/jpeg") && ($_FILES["uploaded_file"]["size"] < 350000)){

//create the directory if doesn't exists (should have write permissons)
if(!is_dir("./files")) mkdir("./files", 0755); 
//move the uploaded file
move_uploaded_file($_FILES['Filedata']['tmp_name'], "./files/".$_FILES['Filedata']['name']);
chmod("./files/".$_FILES['Filedata']['name'], 0777);
}
?>

[sps]

I am sure I am missing something, but where do you make the switch from $_FILES['Filedata'] (in your original script) to $_FILES["uploaded_file"] in your if() statement?  Should these not match?  Which name is the flash form posting to? 

 

What shows up when you do a print_r($_FILES); ?

[foevah]

yes they should but I tried changing it to Filedata and it still doesn't work!

 

["uploaded_file"]

 

should it be ['Filedata'] ?

 

I just changed that line trying with "Filedata" and also I tried change it 'Filedata':

if (($_FILES['Filedata']['type'] == "image/jpeg") && ($_FILES['Filedata']['size'] < 350000)){

 

this still doesn't work..

[foevah]

when I did print_r($_FILES); I got Array ( )

http://www.webdesignhull.lincoln.ac.uk/jecgardner/flash_upload/upload.php

[sps]

Did you try the print_r($_FILES); after uploading a file or just by going to the upload.php page itself?  It should have some info in it after a file upload.  Your flash uploader won't show the print_r() data so it may be best to try with a simple HTML form for testing (or dump the $_FILE array to a flat log file so you can see what is going on.)

 

And if ['Filedata'] works without the IF clause then that is what you need to use in the if clause, not ["uploaded_file"] (unless that is what the flash uploader is naming it.)

 

Other than that discrepancy I don't see why it wouldn't work (maybe the flash uploader isn't setting the mime type correctly?) 

[foevah]

I added print_r($_FILES); at the end of the php code.

 

 

this is the action script

//Allow this domain
System.security.allowDomain("http://localhost/");
import flash.net.FileReference;
// The listener object listens for FileReference events.
var listener:Object = new Object();

// When the user selects a file, the onSelect() method is called, and
// passed a reference to the FileReference object.
listener.onSelect = function(selectedFile:FileReference):Void {
  //clean statusArea and details area
  statusArea.text = details.text = ""
  // Flash is attempting to upload the image.
  statusArea.text += "Attempting to upload " + selectedFile.name + "\n";
  // Upload the file to the PHP script on the server.
  selectedFile.upload("upload.php");
};

// the file is starting to upload.
listener.onOpen = function(selectedFile:FileReference):Void {
  statusArea.text += "Uploading " + selectedFile.name + "\n";
};
//Possible file upload errors
listener.onHTTPError = function(file:FileReference, httpError:Number):Void {
imagePane.contentPath = "error";
imagePane.content.errorMSG.text = "HTTPError number: "+httpError +"\nFile: "+ file.name;
}

listener.onIOError = function(file:FileReference):Void {
imagePane.contentPath = "error";
imagePane.content.errorMSG.text = "IOError: "+ file.name;
}

listener.onSecurityError = function(file:FileReference, errorString:String):Void {
imagePane.contentPath = "error";
imagePane.content.errorMSG.text = "SecurityError: "+SecurityError+"\nFile: "+ file.name;	
}

// the file has uploaded
listener.onComplete = function(selectedFile:FileReference):Void {
  // Notify the user that Flash is starting to download the image.
  statusArea.text += "Upload finished.\nNow downloading " + selectedFile.name + " to player\n";
  //Show file details
  details.text = ""
  for(i in selectedFile) details.text +="<b>"+i+":</b> "+selectedFile[i]+"\n"
  // Call the custom downloadImage() function.
  downloadImage(selectedFile.name);
};

var imageFile:FileReference = new FileReference();
imageFile.addListener(listener);

uploadBtn.onPress = uploadImage;
imagePane.addEventListener("complete", imageDownloaded);

// Call the uploadImage() function, opens a file browser dialog.
function uploadImage(event:Object):Void {
  imageFile.browse([{description: "Image Files", extension: "*.jpg;*.gif;*.png"}]);
}

// If the image does not download, the event object's total property
// will equal -1. In that case, display am error message
function imageDownloaded(event:Object):Void {
  if(event.total == -1) {
    imagePane.contentPath = "error";	
  }
}

// show uploaded image in scrollPane
function downloadImage(file:Object):Void {
  imagePane.contentPath =  "./files/" + file;
}

stop()

[sps]

You definitely want that if() line to be -

 

if (($_FILES['Filedata']['type'] == "image/jpeg") && ($_FILES['Filedata']['size'] < 350000)){

 

(as the flash uploader names it as such, not "uploaded_file")

 

Do you have access to the php error_log file?  It may give you a clue.  The print_r() won't help using the flash file as an uploader as it never redirects to that page for you to view it.  You are saying it is allowing gif files to be uploaded even with the if statement in place?  My guess would be that the flash uploader is not setting the mime type correctly then. 

 

If you only want to allow jpegs you might as well change -

 

imageFile.browse([{description: "Image Files", extension: "*.jpg;*.gif;*.png"}]);

 

to -

 

imageFile.browse([{description: "Image Files", extension: "*.jpg"}]);

 

in your action script.  (Sill need to check it with PHP though.)

 

[foevah]

I want it to accept all images but in the php code I have jpeg..

 

I tried what you suggested which I already thought about trying changing this line in the AS but it still doesn't work:

imageFile.browse([{description: "Image Files", extension: "*.jpg"}]);

 

This is the flash upload is on this link now:

http://www.webdesignhull.lincoln.ac.uk/jecgardner/flash_upload/upload.swf

files go to:

http://www.webdesignhull.lincoln.ac.uk/jecgardner/flash_upload/files/

 

I'm trying to upload a jpeg which it doesn't do.. If I delete the code below the uploads work insecurely:

if (($_FILES['Filedata']['type'] == "image/jpeg") && ($_FILES['Filedata']['size'] < 350000)){

[foevah]

Do you have access to the php error_log file?  It may give you a clue. 

 

I dont think I have access to a php error_log file

 

You are saying it is allowing gif files to be uploaded even with the if statement in place?

 

When the first if statement is in place nothing works.

 

  My guess would be that the flash uploader is not setting the mime type correctly then. 

 

I don't know what mime type is..

 

I have another example of a flash uploader which is giving me the exact some problem! Right now it works because I have deleted that line of code.

Heres the link for this other attempt:

http://www.jamesgardner.lincoln.ac.uk/fmp/fileupload/

The file for this can be found in your macromedia flash 8 directory

Macromedia\Flash 8\Samples and Tutorials\Samples\ActionScript\FileUpload

[sps]

My guess again is that the flash uploader is not setting the MIME type correctly (the "image/jpeg" part)

 

Try changing -

 

if (($_FILES['Filedata']['type'] == "image/jpeg") && ($_FILES['Filedata']['size'] < 350000)){

 

to just the size check -

 

if ($_FILES['Filedata']['size'] < 350000){

 

If it works then we can at least narrow it down to a MIME type issue.

 

Edit:

 

It appears files uploaded with flash always have a MIME type of "application/octet-stream" (regardless of what type of file it is.)  So that appears to be why your IF statement is failing.  I would just use the size check in your IF statement, and check that the file extension is .jpg or .jpeg, etc. in PHP. 

[foevah]

yayaya it works

 

http://www.webdesignhull.lincoln.ac.uk/jecgardner/flash_upload/upload.swf

 

Ok so I deleted it checking for jpeg's so how can I get it to work so it only accepts images?

[sps]

Since you only allow .jpg .gif and .png in your flash file I would just try something like this -

 

if ((substr(basename($_FILES['Filedata']['name'],-4) == '.jpg' || substr(basename($_FILES['Filedata']['name'],-4) == '.gif' || substr(basename($_FILES['Filedata']['name'],-4) == '.png') && ($_FILES['Filedata']['size'] < 350000)) {

 

Note that all this does is make sure the last 4 characters of the file name are .jpg or .gif or .png, it can not verify that the data in a file named as such is actually a valid graphic.  (i.e. you could rename a .exe file to .jpg and it would still upload.)  But as MIME types can be faked anyway I don't see this as much of an issue (compared to the "image/jpeg" check).  If you really want to verify that the data is a valid graphic you may want to open the files with GD functions and save them using GD functions, any error in that process would signal invalid data. 

[foevah]

I got this error

Parse error: parse error, unexpected '{' in /home/hullweb/public_html/jecgardner/flash_upload/upload.php on line 4

[sps]

Whoops, try -

 

if ((substr(basename($_FILES['Filedata']['name'],-4)) == '.jpg' || substr(basename($_FILES['Filedata']['name'],-4)) == '.gif' || substr(basename($_FILES['Filedata']['name'],-4)) == '.png') && ($_FILES['Filedata']['size'] < 350000)) {

[foevah]

 

Warning: Wrong parameter count for substr() in /home/hullweb/public_html/jecgardner/flash_upload/upload.php on line 3 ???

[sps]

HA, sorry, how about -

 

if ((substr(basename($_FILES['Filedata']['name']),-4,4) == '.jpg' || substr(basename($_FILES['Filedata']['name']),-4,4) == '.gif' || substr(basename($_FILES['Filedata']['name']),-4,4) == '.png') && ($_FILES['Filedata']['size'] < 350000)) {

 

 

[foevah]

sweet thanks very much it works!!!!

[foevah]

wasn't I checking the file type before?