Be careful with file uploads

Published on and tagged with cakephp  problem  tip

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!

8 comments baked

  • Madarco

    Hi, isn’t possible to use is_uploaded_file on params['form']['File']['tmp_name']?

  • cakebaker

    Yes, it is possible to use is_uploaded_file.

  • Lamby

    :-)

  • cakebaker » File upload with CakePHP

    [...] Update (2006-08-05): Fixed a security hole in the code above, see also “Be careful with file uploads”. Thanks to Lamby. [...]

  • Joker-eph

    Seems to be just a bad hack… The security hole still remains :

    <input type="hidden" name="File[tmp_name]"
                     value="/tmp/../home/dho/projectA/app/config/database.php" />
  • cakebaker

    @Joker-eph: Thanks for the hint, it is now fixed in the article!

  • fairuz

    how to make multi file upload. see my code below

    <?php
    // my view ... add.ctp
    $this->Form->create('Post',array('action'=>'add','type'=>'file'));
    $this->Form->file('file1');
    $this->Form->file('file2');
    $this->Form->submit('Upload ');
    $this->Form->end();
    ?>

    how to create controller for multiple files?

  • cakebaker

    @fairuz: You can access the file data in your controller in the following way:

    $this->data['Post']['file1']['name'];
    $this->data['Post']['file2']['name'];

    However, a more flexible solution is to modify your view slightly:
    ...
    $this->Form->file('Post.0.File');
    $this->Form->file('Post.1.File');
    ...

    You can then access the data like:
    $this->data['Post'][0]['File']['name'];
    $this->data['Post'][1]['File']['name'];

    This approach makes it easier to loop over the files.

    I hope this answers your question.

Bake a comment




(for code please use <code>...</code> [no escaping necessary])

© daniel hofstetter. Licensed under a Creative Commons License