#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 Brian Bouterse):
I think I see why some installations broke with this but other's haven't.
Look at this reproducer:
{{{
from django.core.files import File
from django.db import models
class MyModelWithUploadTo(models.Model):
def upload_to_func(self, name):
return 'qqq'
file = models.FileField(upload_to=upload_to_func)
class MyModel(models.Model):
file = models.FileField()
with open('/tmp/somefile', 'w') as f:
f.write('asdfasdfasdf')
f.flush()
with open('/tmp/anotherfile', 'w') as f:
f.write('fsfoisufsiofdsoiufd')
f.flush()
model_instance_with_upload_to = MyModelWithUploadTo()
model_instance_with_upload_to.file = File(open('/tmp/somefile', 'rb'))
model_instance_with_upload_to.save()
print('I saved the one with upload_to()\n\n')
model_instance = MyModel()
model_instance.file = File(open('/tmp/anotherfile', 'rb'))
model_instance.save()
}}}
On 2.2.20 when I makemigrations for those two models, apply them, and the
run the reproducer I get:
{{{
I saved the one with upload_to()
Traceback (most recent call last):
File "/home/vagrant/devel/django_file_name_2_2_21_reproducer.py", line
30, in <module>
model_instance.save()
File "/usr/local/lib/pulp/lib64/python3.9/site-
packages/django/db/models/base.py", line 743, in save
self.save_base(using=using, force_insert=force_insert,
File "/usr/local/lib/pulp/lib64/python3.9/site-
packages/django/db/models/base.py", line 780, in save_base
updated = self._save_table(
File "/usr/local/lib/pulp/lib64/python3.9/site-
packages/django/db/models/base.py", line 873, in _save_table
result = self._do_insert(cls._base_manager, using, fields, update_pk,
raw)
File "/usr/local/lib/pulp/lib64/python3.9/site-
packages/django/db/models/base.py", line 910, in _do_insert
return manager._insert([self], fields=fields, return_id=update_pk,
File "/usr/local/lib/pulp/lib64/python3.9/site-
packages/django/db/models/manager.py", line 82, in manager_method
return getattr(self.get_queryset(), name)(*args, **kwargs)
File "/usr/local/lib/pulp/lib64/python3.9/site-
packages/django/db/models/query.py", line 1186, in _insert
return query.get_compiler(using=using).execute_sql(return_id)
File "/usr/local/lib/pulp/lib64/python3.9/site-
packages/django/db/models/sql/compiler.py", line 1376, in execute_sql
for sql, params in self.as_sql():
File "/usr/local/lib/pulp/lib64/python3.9/site-
packages/django/db/models/sql/compiler.py", line 1318, in as_sql
value_rows = [
File "/usr/local/lib/pulp/lib64/python3.9/site-
packages/django/db/models/sql/compiler.py", line 1319, in <listcomp>
[self.prepare_value(field, self.pre_save_val(field, obj)) for field in
fields]
File "/usr/local/lib/pulp/lib64/python3.9/site-
packages/django/db/models/sql/compiler.py", line 1319, in <listcomp>
[self.prepare_value(field, self.pre_save_val(field, obj)) for field in
fields]
File "/usr/local/lib/pulp/lib64/python3.9/site-
packages/django/db/models/sql/compiler.py", line 1270, in pre_save_val
return field.pre_save(obj, add=True)
File "/usr/local/lib/pulp/lib64/python3.9/site-
packages/django/db/models/fields/files.py", line 288, in pre_save
file.save(file.name, file.file, save=False)
File "/usr/local/lib/pulp/lib64/python3.9/site-
packages/django/db/models/fields/files.py", line 87, in save
self.name = self.storage.save(name, content,
max_length=self.field.max_length)
File "/usr/local/lib/pulp/lib64/python3.9/site-
packages/django/core/files/storage.py", line 52, in save
return self._save(name, content)
File "/home/vagrant/devel/pulpcore/pulpcore/app/models/storage.py", line
46, in _save
full_path = self.path(name)
File "/usr/local/lib/pulp/lib64/python3.9/site-
packages/django/core/files/storage.py", line 323, in path
return safe_join(self.location, name)
File "/usr/local/lib/pulp/lib64/python3.9/site-
packages/django/utils/_os.py", line 44, in safe_join
raise SuspiciousFileOperation(
django.core.exceptions.SuspiciousFileOperation: The joined path
(/tmp/anotherfile) is located outside of the base path component
(/var/lib/pulp/media)
}}}
Notice how the output includes the `I saved the one with upload_to()`.
That shows this was working with 2.2.20.
Now run that same reproducer, migrations, etc against 2.2.21. I see:
{{{
Traceback (most recent call last):
File "/home/vagrant/devel/django_file_name_2_2_21_reproducer.py", line
25, in <module>
model_instance_with_upload_to.save()
File "/usr/local/lib/pulp/lib64/python3.9/site-
packages/django/db/models/base.py", line 743, in save
self.save_base(using=using, force_insert=force_insert,
File "/usr/local/lib/pulp/lib64/python3.9/site-
packages/django/db/models/base.py", line 780, in save_base
updated = self._save_table(
File "/usr/local/lib/pulp/lib64/python3.9/site-
packages/django/db/models/base.py", line 873, in _save_table
result = self._do_insert(cls._base_manager, using, fields, update_pk,
raw)
File "/usr/local/lib/pulp/lib64/python3.9/site-
packages/django/db/models/base.py", line 910, in _do_insert
return manager._insert([self], fields=fields, return_id=update_pk,
File "/usr/local/lib/pulp/lib64/python3.9/site-
packages/django/db/models/manager.py", line 82, in manager_method
return getattr(self.get_queryset(), name)(*args, **kwargs)
File "/usr/local/lib/pulp/lib64/python3.9/site-
packages/django/db/models/query.py", line 1186, in _insert
return query.get_compiler(using=using).execute_sql(return_id)
File "/usr/local/lib/pulp/lib64/python3.9/site-
packages/django/db/models/sql/compiler.py", line 1376, in execute_sql
for sql, params in self.as_sql():
File "/usr/local/lib/pulp/lib64/python3.9/site-
packages/django/db/models/sql/compiler.py", line 1318, in as_sql
value_rows = [
File "/usr/local/lib/pulp/lib64/python3.9/site-
packages/django/db/models/sql/compiler.py", line 1319, in <listcomp>
[self.prepare_value(field, self.pre_save_val(field, obj)) for field in
fields]
File "/usr/local/lib/pulp/lib64/python3.9/site-
packages/django/db/models/sql/compiler.py", line 1319, in <listcomp>
[self.prepare_value(field, self.pre_save_val(field, obj)) for field in
fields]
File "/usr/local/lib/pulp/lib64/python3.9/site-
packages/django/db/models/sql/compiler.py", line 1270, in pre_save_val
return field.pre_save(obj, add=True)
File "/usr/local/lib/pulp/lib64/python3.9/site-
packages/django/db/models/fields/files.py", line 289, in pre_save
file.save(file.name, file.file, save=False)
File "/usr/local/lib/pulp/lib64/python3.9/site-
packages/django/db/models/fields/files.py", line 87, in save
name = self.field.generate_filename(self.instance, name)
File "/usr/local/lib/pulp/lib64/python3.9/site-
packages/django/db/models/fields/files.py", line 303, in generate_filename
filename = validate_file_name(filename)
File "/usr/local/lib/pulp/lib64/python3.9/site-
packages/django/core/files/utils.py", line 8, in validate_file_name
raise SuspiciousFileOperation("File name '%s' includes path elements"
% name)
django.core.exceptions.SuspiciousFileOperation: File name '/tmp/somefile'
includes path elements
}}}
Here the `SuspiciousFileOperation` is also raised on the saving of
`MyModelWithUploadTo`. Do you all know why this difference is significant?
Regarding non reproducer discussion...
Yes let's take how many folks are broken out of the equation; we have to
do what's secure, even if its inconvenient. I agree with that, and thank
you for saying that. I think what is contributing to the issue is
confusion on why these 2 lines are necessary given how the CVE reads. I'm
not here to second guess the good work of the security response team or
it's review process, but I am confused.
As a practical matter, I'm still trying to figure out if another release
removing these two lines will occur, or if the CVE description needs
revision. Do you have some advice for me on which you think will happen?
Also I'd like to exchange perspectives on version pinning for shipped
django apps depending on the Django LTS, but not until we resolve the
matter at hand first.
Thank you for everything. Our project and users really appreciate it.
Please let me know how we can help.
--
Ticket URL: <https://code.djangoproject.com/ticket/32718#comment:21>
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 django-updates+unsubscr...@googlegroups.com.
To view this discussion on the web visit
https://groups.google.com/d/msgid/django-updates/064.d454cc6e13c5f7e71e393459ddc580c7%40djangoproject.com.
