开发者

PHP Image Gallery (Upload Problem)

This is probably a simple question, however I am finding it difficult to accomplish.

I have a php class ("class.upload.php" code is below*) that is called with:

<?php 
    $upload = new upload(); 
    $upload->upload_file();
?>

so in a test page it would be like so:

<?php 
    $upload = new upload(); 
    $upload->upload_file(); 
?>


<form action="" method="post" enctype="multipart/form-data">
    <input type="file" id="real_upload" class="hide" name="file" />
    <input type="submit" id="real_submit" class="hide" value="Upload" />
</form>

The problem is I'm using this upload system for parts of the website, what I want is to have this class upload the file once it has gone through the "gallery upload" segment:

    <?php

    $mysql_link = mysql_connect("localhost", "", "");   
        mysql_select_db("") or die("Could not select database");

        while($counter <= count($photos_uploaded)) {
            if($photos_uploaded['size'][$counter] > 0) {
                if(!array_key_exists($photos_uploaded['type'][$counter], $known_photo_types)) {
                    $result_final .= "File ".($counter+1)." is not a photo<br />";
                }
                else {
                    mysql_query( "INSERT INTO gallery_photos(`photo_filename`, `photo_caption`, `photo_category`) VALUES('0', '".addslashes($photo_caption[$counter])."', '".addslashes($_POST['category'])."')" );
                    $new_id = mysql_insert_id();
                    $filetype = $photos_uploaded['type'][$counter];
                    $extention = $known_photo_types[$filetype];
                    $filename = $new_id.".".$extention;

                    mysql_query( "UPDATE gallery_photos SET photo_filename='".addslashes($filename)."' WHERE photo_id='".addslashes($new_id)."'" );
 }

    ?>

How would i approach this?

Thanks, Keiran

class.upload.php

<?php
class Upload {
    //File Max Size:
    protected $max_file_size = 5;

    public function upload_file() {
        //Check for upload request:
        if(isset($_FILES['file'])) {
            //Set File Information:
            $file = array(
                'name' => $_FILES['file']['name'],
                'type' => $_FILES['file']['type'],
                'size' => $_FILES['file']['size'],
                'temp' => $_FILES['file']['tmp_name'],
                'error' => $_FILES['file']['error']
            );

            //Check if it is under the max size limit
            if($file['size'] < ($this->max_file_size * 1048576)) {

            //Filename:
                $filename = strtolower($file['name']);
                $filename = str_replace(" ","_",$filename);

                //Check for a custom path location, if none exists it will load into a file associated directory
                if (isset($_REQUEST['customPath'])) {
                    $path = 'uploads/'.$_REQUEST['customPath'].'/';
                } else {
                    $path = 'uploads/'.$file['type'].'/';
                        if(!file_exists($path) && !is_dir($path)) {
                            mkdir($path, 开发者_StackOverflow中文版"511", true);
                        }
                }

                //Now lets more the file:
                $move_file = move_uploaded_file($file['temp'], $path . $filename);

                if($move_file) {
                    echo 'uploads/'.$filename;
                }
            } else {
                echo 'Your file is too big to upload to our server.';
            }
        }
    }
}

?>


Your upload class is opening your server to a complete compromise.

  1. You do not sanitize the customPath field, and blindly use it as part of the "where do I put this file" path. A malicious user can enter a relative path and your script will happily attempt to write the uploaded file ANYWHERE on your server
  2. You do not check for filename collisions, so a malicious user can combine vulnerability #1 with this and your script will happily overwrite any file the webserver has access to. While (I hope) your server isn't running as root, consider a customPath of ../../../../../../../../../../etc/ and an uploaded filename of passwd, leading your webserver to overwrite /etc/passwd with a user-supplied version.
  3. The size parameter in the $_FILES array is also user-supplied and can be trivially forged. A malicious user can set it to 1 and upload a multi-terabyte file if they so choose. It is better to determine actual filesize with filesize($_FILES['file']['tmp_name']) instead.
  4. Checking isset($_FILES['file']) is not sufficient to determine if a file was actually uploaded. The file entry will be recreated ANY TIME an <input type="file" name="file"> is present within the form, whether the upload succeeded or not. You have to check ($_FILES['file']['error'] === UPLOAD_ERR_OK) to see if the upload was sucessful or not (the error constants are defined here)
  5. The type portion of the $_FILES array is also user-supplied and can be subverted. Nothing prevents a malicious user from setting it to image/jpeg but still uploading nasty_virus_from_hell.exe to your server. You have to use getimagesize() or FileInfo() to determine the mime type yourself in a secure manner.
  6. addslashes() is a very poor method of sanitizing data for SQL query insertion. It is NOT a secure method, use mysql_real_escape_string(), which uses MySQL's own API to do sanitization, and do so in a way guaranteed to be "safe" for MySQL.
0

上一篇:

下一篇:

精彩评论

暂无评论...
验证码 换一张
取 消

最新问答

问答排行榜