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.

sidewinder
  • 2,253
  • 6
  • 21
  • 33
  • 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 '11 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 '11 at 10:07
  • use os.path.join(); better than string concatenation. – Matt Feifarek Jul 27 '11 at 16:18

1 Answers1

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.

Sean Vieira
  • 140,251
  • 31
  • 286
  • 277