#32718: [3.2.1] Issue with assigning file to FileField
-------------------------------------+-------------------------------------
     Reporter:  Jakub Kleň           |                    Owner:  nobody
         Type:  Bug                  |                   Status:  new
    Component:  Database layer       |                  Version:  2.2
  (models, ORM)                      |
     Severity:  Release blocker      |               Resolution:
     Keywords:  3.2.1 file model     |             Triage Stage:  Accepted
  filefield fieldfile                |
    Has patch:  1                    |      Needs documentation:  1
  Needs tests:  1                    |  Patch needs improvement:  1
Easy pickings:  0                    |                    UI/UX:  0
-------------------------------------+-------------------------------------

Comment (by Jakub Kleň):

 Replying to [comment:22 carderm]:
 > I believe an absolute path outside of Media Root should fail, that is
 correct (in my opinion) - as an **absolute** path might be able to write
 shell scripts around the file system...
 >
 > It should however, work for a **relative** file path.
 >
 > **Hack example here**
 >
 > {{{
 > # Some model
 > class FileModel(models.Model):
 >     file_field = models.FileField(_('UsersFile'), storage='users',
 max_length=1000)
 >
 > model_inst = FileModel()
 >
 > # Add some file
 > new_file = ContentFile(some_file.read())
 > new_file.name = f"{user.name}/file.txt"    # E.g. "steve/file.txt"   <<<
 This file should only be for 'steve'
 > model_inst.file = new_file
 >
 > # Save the Model
 > model_inst.save()    #<<< in Django 3.2.0 this creates file
 "MEDIA_ROOT/users/steve/file.txt   ! which is correct
 > model_inst.save()    #<<< in Django 3.2.1 this FAILS with Path issue !
 Incorrect
 > model_inst.save()    #<<< in Django Latest Git commit will create
 "MEDIA_ROOT/users/file.txt ! which is incorrect - missing path!
 >
 > # What we need to stop:
 > bad_file = ContentFile(malicious_script.read())
 > bad_file.name = "/etc/init.d/nameofscript.sh"
 > # or
 > bad_file.name = "../../../../../usr/local/lib/bad.file"
 > model_inst.file = bad_file
 >
 > # On Save we need to protect here - imho
 > model_inst.save()     # <<< This *should Fail* with Security Error
 >
 > }}}
 >
 >
 >
 >
 >
 >

 Absolute paths wouldn't be able to write shell scripts around the file
 system, because the path always starts with `MEDIA_ROOT`, and `../` and
 such are disallowed.

 Also, I'm not sure if we should be forced to override the `name` attribute
 of `File`, because overriding it breaks the `File.open` function from
 working properly.

-- 
Ticket URL: <https://code.djangoproject.com/ticket/32718#comment:27>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/064.f2c9153172481dcc4b0f1b4efbbda1ba%40djangoproject.com.

Reply via email to