Security issue regarding email file attachments

[nikess]

from https://discuss.frappe.io/t/email-file-attachment-security/3812 👍

If I email a customer using Support Ticket, and attach a document by uploading it first, that document appears to be publicly available, via /files/. , even if you're not authenticated at all. (at least when running via bench start).

This is of course completely unworkable - that document could be sensitive.

[pdvyas]

Plan to fix this,

• For a request to /private/files, get File Data records for the path and check if user has read perms for attached doc for any of them
• Cache as much as possible as perm checking could be expensive
• Move filesfrom public/files to private/files and change urls (patch)

if not os.path.exist(path):
    return 404
fd = frappe.get_doc("File Data", { "file_url": path })
if not fd:
    return 404
if fd.refdoctype in ('Web Page', 'Blog Post'):
    serve_file()
if frappe.local.session.user == "Guest":
    return 403
if has_permission(fd.refdoctype, doc=fd.refdocname, ptype="read"):
    serve_file()

[pdvyas]

On second thought, I think private files should stay in private/files and public in public/files. More than 50% of the requests are for public files, so it's better to serve them without hitting the app server/database.

[pdvyas]

Another cheap solution would be to use links that expire every 30s. So, the extra cost of serving a private file is just some math. (like Amazon S3)

[rmehta]

@pdvyas How will you differentiate?

[pdvyas]

A check in DocType, "has private files".
Also, we'll do this for only new files. (as discussed)

[djagtap]

Can we introduce more robust file management capabilities?
while creating any DocType, if there are any attach type is used then its storage location, download and view/display method, should be customizable from create DocType form itself or by separate module which will allow such customization on per DocType level if necessary.

Ref: http://stackoverflow.com/questions/2687957/django-serving-media-behind-custom-url
http://stackoverflow.com/questions/9897050/url-design-ways-to-hide-pk-id-from-url

[pdvyas]

We already use X-Accel-redirect to serve backups.

For your suggestion, what use case would it cover? We already store
reference to the doc in File Data table. If you want to add more
validations, you can add hooks to File Data.

On Wed, Nov 26, 2014 at 12:37 PM, Dinesh Jagtap notifications@github.com
wrote:

Can we introduce more robust file management capabilities?
while creating any DocType, if there are any attach type is used then its
storage location, download and view/display method, should be customizable
from create DocType form itself or by separate module which will allow such
customization on per DocType level if necessary.

Ref:
http://stackoverflow.com/questions/2687957/django-serving-media-behind-custom-url

—
Reply to this email directly or view it on GitHub
#927 (comment).

[justinlusg]

After the update, the private/files folder is not accessible even when I'm logged on. I get this error when I clicked on the link itself within the ERP (e.g. NOT pasting the link)

[justinlusg]

@anandpdoshi Reference to frappe/erpnext#4474

[anandpdoshi]

Do

bench update
bench setup nginx
sudo service nginx reload

Sent from my phone

On 10-Dec-2015, at 8:50 PM, justinlusg notifications@github.com wrote:

Reference to frappe/erpnext#4474

—
Reply to this email directly or view it on GitHub.

[justinlusg]

Works now, thanks!