Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Doctrine] Security issue in handling file uploads with Doctrine #4177

Closed
Mak-Di opened this issue Aug 25, 2014 · 3 comments
Closed

[Doctrine] Security issue in handling file uploads with Doctrine #4177

Mak-Di opened this issue Aug 25, 2014 · 3 comments
Labels
actionable Clear and specific issues ready for anyone to take them. Doctrine good first issue Ideal for your first contribution! (some Symfony experience may be required) hasPR A Pull Request has already been submitted for this issue.

Comments

@Mak-Di
Copy link

Mak-Di commented Aug 25, 2014

Doc file

protected function getUploadRootDir()
    {
        // the absolute directory path where uploaded
        // documents should be saved
        return __DIR__.'/../../../../web/'.$this->getUploadDir();
    }

I don't think that storing the upload file inside of document root is a good idea.

@stof
Copy link
Member

stof commented Aug 27, 2014

Storing the file in the document root is done on purpose so that it can be accessed by clients.

If your files need to be secured, the logic needs to be different indeed (and then using a controller to access them), but this cookbook entry describes the simple case, not more complex cases

@Mak-Di
Copy link
Author

Mak-Di commented Aug 27, 2014

Agree, but maybe it is useful to add a note that storing the upload file inside of document root is not secure because many think that cookbook shows the best practice.

@wouterj
Copy link
Member

wouterj commented May 1, 2015

I'm 👍 for adding a small caution to the article. Something like:

.. caution::

    Be aware that this will save the file inside the document root, which
    can be accessed by everyone. Consider placing it out of the document
    root and adding custom viewing logic when you need to secure the files.

Btw, @Mak-Di, the article is far from a best practice (see also #2346 ). We would be very happy if anyone can turn it into a best practice article :)

@wouterj wouterj added good first issue Ideal for your first contribution! (some Symfony experience may be required) actionable Clear and specific issues ready for anyone to take them. labels May 1, 2015
anacicconi pushed a commit to anacicconi/symfony-docs that referenced this issue May 23, 2015
| Doc fix?      | yes
| New docs?     | no
| Applies to    | all
| Fixed tickets | symfony#4177 [Doctrine] Security issue in handling file uploads with Doctrine
@wouterj wouterj added the hasPR A Pull Request has already been submitted for this issue. label May 23, 2015
weaverryan added a commit that referenced this issue Jun 11, 2015
…coni)

This PR was merged into the 2.3 branch.

Discussion
----------

Add a caution to the getUploadRootDir Doctrine

| Q             | A
| ------------- | ---
| Doc fix?      | yes
| New docs?     | no
| Applies to    | all
| Fixed tickets | #4177

Commits
-------

a80d669 Add a caution to the getUploadRootDir - correction
69475d0 Adding a caution to the getUploadRootDir() method
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
actionable Clear and specific issues ready for anyone to take them. Doctrine good first issue Ideal for your first contribution! (some Symfony experience may be required) hasPR A Pull Request has already been submitted for this issue.
Projects
None yet
Development

No branches or pull requests

5 participants