SitePoint Sponsor

User Tag List

Results 1 to 8 of 8
  1. #1
    SitePoint Wizard
    Join Date
    Feb 2007
    Location
    Southern California
    Posts
    1,388
    Mentioned
    1 Post(s)
    Tagged
    0 Thread(s)

    code check: file upload

    I put together this code from my research. It's the file used to accept a file upload. Is this correct? I am attempting to upload to the page but it is failing. Not sure if the problem is on the server side (this script) or not, at this point. (This code is being used to accept camera shot uploads o smartphones and tablets. I'm using Cordova PhoneGap software.)

    I'm uncertain of the line that goes: "/srv/www/uploads/" This is from an example online. Is /srv/ common to servers, or am I supposed to change it? My server reflects the /www/uploads/ part correctly.

    Code:
    <?php
    define("UPLOAD_DIR", "/srv/www/uploads/"); // UNCERTAIN OF THIS PART: /SRV/
    
    print_r($_FILES);
    
    $allowed_exts = array("gif", "jpeg", "jpg", "png");
    $temp = explode(".", $_FILES["file"]["name"]);
    $extension = end($temp);
    if ((($_FILES["file"]["type"] == "image/gif")
    || ($_FILES["file"]["type"] == "image/jpeg")
    || ($_FILES["file"]["type"] == "image/jpg")
    || ($_FILES["file"]["type"] == "image/pjpeg")
    || ($_FILES["file"]["type"] == "image/x-png")
    || ($_FILES["file"]["type"] == "image/png"))
    && ($_FILES["file"]["size"] < 20000)
    && in_array($extension, $allowed_exts)) {
      if ($_FILES["file"]["error"] > 0) { ?>
    document.getElementById('camera_status').innerHTML = "Return Code: "
    <?php . $_FILES["file"]["error"] . ?>
    document.getElementById('camera_status').innerHTML = "<br/>";
    <?php
      }
      else {
      ?>
      
    document.getElementById('camera_status').innerHTML = "Upload: "
    <?php . $_FILES["file"]["name"] . ?>
    document.getElementById('camera_status').innerHTML =+ "<br/>Type: "
    <?php . $_FILES["file"]["type"] . ?>
    document.getElementById('camera_status').innerHTML =+ "<br/>Size: "
    <?php . ($_FILES["file"]["size"] / 2048) . ?>
    document.getElementById('camera_status').innerHTML =+ " kB<br/>Temp file: "
    <?php . $_FILES["file"]["tmp_name"] . ?>
    document.getElementById('camera_status').innerHTML =+ "<br/>";
    <?php
        if (file_exists(UPLOAD_DIR . $_FILES["file"]["name"])) { ?>
    document.getElementById('camera_status').innerHTML =+ <?php $_FILES["file"]["name"] . ?>
    document.getElementById('camera_status').innerHTML =+ " already exists. ";
    <?php
        }
        else {
          var now = new Date();
          $new_image_name = now + ".jpg";
          move_uploaded_file($_FILES["file"]["tmp_name"], UPLOAD_DIR . $new_image_name);
        }
      }
    }
    else { ?>
    document.getElementById('camera_status').innerHTML =+ "Invalid file";
    <?php 
    }
    ?>
    Steve Husting

  2. #2
    Programming Team silver trophybronze trophy
    Mittineague's Avatar
    Join Date
    Jul 2005
    Location
    West Springfield, Massachusetts
    Posts
    17,264
    Mentioned
    196 Post(s)
    Tagged
    2 Thread(s)
    I'm surprised you're not getting any error messages that might help, but I think you're probably right about the upload directory.

    I don't know how many servers have this structure, but if you don't I think you should create it or change the CONSTANT value.
    It looks to be "outside the root" which might mean the script relies on that for security and it may be vunerable if you change to one under the root.

  3. #3
    Utopia, Inc. silver trophy
    ScallioXTX's Avatar
    Join Date
    Aug 2008
    Location
    The Netherlands
    Posts
    9,097
    Mentioned
    153 Post(s)
    Tagged
    2 Thread(s)
    My guess is also that the directory must be changed to reflect the directory on your server. Other than that there are two quite crucial mistakes in the code. You have a few lines like

    Code:
    document.getElementById('camera_status').innerHTML =+ "<br/>Type: "
    that must be

    Code:
    document.getElementById('camera_status').innerHTML += "<br/>Type: "
    Also, to go from bytes to kB, you need to divide by 1024, not 2048 !

    Lastly, the script isn't very secure, since the file type in $_FILES isn't very reliable. You'd better haul the file through fileinfo and check the information from that, or even better open and save again with GD to make sure that 1) it is in image and 2) all extra data the user may try to piggy back in via the image will be removed.
    Rémon - Hosting Advisor

    SitePoint forums will switch to Discourse soon! Make sure you're ready for it!

    Minimal Bookmarks Tree
    My Google Chrome extension: browsing bookmarks made easy

  4. #4
    SitePoint Wizard
    Join Date
    Feb 2007
    Location
    Southern California
    Posts
    1,388
    Mentioned
    1 Post(s)
    Tagged
    0 Thread(s)
    Thank you for the valuable info! I am using BlueHost, so I don't think I can do anything with fileinfo or GD. But I made the other changes you pointed out.

    Also, I'll research to figure out how to make it more secure. Ideas welcome!
    Steve Husting

  5. #5
    SitePoint Wizard
    Join Date
    Feb 2007
    Location
    Southern California
    Posts
    1,388
    Mentioned
    1 Post(s)
    Tagged
    0 Thread(s)
    Looks like I can replace this ...
    Code:
    $allowed_exts = array("jpeg", "jpg");
    $temp = explode(".", $_FILES["file"]["name"]);
    $extension = end($temp);
    if ((($_FILES["file"]["type"] == "image/jpeg")
    || ($_FILES["file"]["type"] == "image/jpg"))
    && ($_FILES["file"]["size"] < 20000)
    && in_array($extension, $allowed_exts)) {
      if ($_FILES["file"]["error"] > 0) { ?>document.getElementById('camera_status').innerHTML += "Return Code: "
    <?php . $_FILES["file"]["error"] . ?>document.getElementById('camera_status').innerHTML += "<br/>";
    ... with this ...
    Code:
    $allowedTypes = array(IMAGETYPE_PNG, IMAGETYPE_JPEG, IMAGETYPE_GIF);
    $detectedType = exif_imagetype($_FILES['file']['name']);
    $error = !in_array($detectedType, $allowedTypes);
      ?>
      document.getElementById('camera_status').innerHTML += "Upload failed. Try again."
    ... for more security
    Steve Husting

  6. #6
    Utopia, Inc. silver trophy
    ScallioXTX's Avatar
    Join Date
    Aug 2008
    Location
    The Netherlands
    Posts
    9,097
    Mentioned
    153 Post(s)
    Tagged
    2 Thread(s)
    Yup, that would work!
    Rémon - Hosting Advisor

    SitePoint forums will switch to Discourse soon! Make sure you're ready for it!

    Minimal Bookmarks Tree
    My Google Chrome extension: browsing bookmarks made easy

  7. #7
    SitePoint Wizard
    Join Date
    Feb 2007
    Location
    Southern California
    Posts
    1,388
    Mentioned
    1 Post(s)
    Tagged
    0 Thread(s)
    Finally got console output from Eclipse. At fileTransfer, it insists that the upload.php page is not found:

    E/FileTransfer(1013): java.io.FileNotFoundException: <URL here>

    The URL is gives is correct, and if I use that URL in a browser, there is no 404 error. What's odd is that I have this at the very bottom of the php page, and it doesn't show the <h2> text:

    Code:
    }
    ?>
    
    <!DOCTYPE html>
    <html>
        <head>
            <meta charset="utf-8" />
    		<title>upload page</title>
    	</head>
    <body style="background-color:yellow;">
    	<h2>From HTML: This is an upload page.</h2> 
        </body>
    </html>
    And at the top of the page I have the following, and it doesn't display that text either:
    Code:
    <?php
    print("From PHP: This is an upload page.");
    So now I know the problem is my php page somewhere. When I put this in a php page and access it, it gives me all kinds of info on PHP 5.4.24:

    Code:
    <?php phpinfo(); ?>
    Steve Husting

  8. #8
    SitePoint Wizard
    Join Date
    Feb 2007
    Location
    Southern California
    Posts
    1,388
    Mentioned
    1 Post(s)
    Tagged
    0 Thread(s)
    I tried this ...

    define("UPLOAD_DIR", "/www/uploads/");

    and this...

    define("UPLOAD_DIR", $_SERVER['DOCUMENT_ROOT'] . "/www/uploads/");

    No difference in the error.
    Steve Husting


Bookmarks

Posting Permissions

  • You may not post new threads
  • You may not post replies
  • You may not post attachments
  • You may not edit your posts
  •