Be careful with file uploads

In a comment lamby pointed out a possible security hole in the code of my post “File upload with CakePHP”. He is right, if you know the location of app/config/database.php you can retrieve the database settings including the password.

In the following I will show you the original code, the exploit, and a possible solution.

Ok, here is the view from my earlier post:

// app/views/files/add.thtml
<form action="/files/add" enctype="multipart/form-data" method="post">
    <?php echo $html->file('File'); ?>
    <?php echo $html->submit('Upload')); ?>
</form>

And here is the controller:

// app/controllers/files_controller.php
class FilesController extends AppController
{
    function add()
    {
        if (!empty($this->params['form']))
        {
            $fileData = fread(fopen($this->params['form']['File']['tmp_name'], "r"),
                                     $this->params['form']['File']['size']);
            $this->params['form']['File']['data'] = $fileData;

            $this->File->save($this->params['form']['File']);

            $this->redirect('somecontroller/someaction');
        }
    }
}

With the following view we can store the file app/config/database.php in the database (and download it later):

// app/views/files/exploit.thtml
<form action="/files/add" enctype="multipart/form-data" method="post">
    <input type="hidden" name="File[size]" value="9999" />
    <input type="hidden" name="File[tmp_name]"
                 value="/home/dho/projectA/app/config/database.php" />
    <?php echo $html->submit('Upload')); ?>
</form>

To avoid such an attack we have to check in the controller if $this->params['form']['File']['tmp_name'] points to the folder for temporary files to use the PHP function is_uploaded_file to ensure the file was really uploaded:

if (!empty($this->params['form']) && is_uploaded_file($this->params['form']['File']['tmp_name'])

Update 2008-10-22: Fixing security hole, thanks to Joker-eph!