14

Currently, I am using the following method for uploading files (via HTML form) in Pyramid.

if request.params.get('form.submitted'):

    upload_directory = os.getcwd() + '/myapp/static/uploads/'

    my_file = request.POST.get('thumbnail')
    saved_file = str(upload_directory) + str(my_file.filename)

    perm_file = open(saved_file, 'w')

    shutil.copyfileobj(my_file.file, perm_file)
    my_file.file.close()
    perm_file.close()

I am just wondering, is this a good way of saving file uploads, are there any security concerns with my method? How else can I improve my method. Thanks.

3
  • 1
    How does this go with large files? I think for large files, you have to write in chunks. I'm going to give your method a try though, thanks!
    – MFB
    Jul 26, 2011 at 23:09
  • Hi, at the moment I have not tried this on large files. Mostly on files a under a meg. But please do report back if you try this on larger files and let me know how it goes, thanks.
    – sidewinder
    Jul 27, 2011 at 10:07
  • use os.path.join(); better than string concatenation. Jul 27, 2011 at 16:18

1 Answer 1

14

You'll want to use something like werkzug's safe_join rather than just adding the upload directory to the given file name. An attacker could create a POST with a filename of ../../../some/important/path and cause this script to overwrite some file outside of your upload_directory.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.