Django 3.2.4 Update: SuspiciousFileOperation ... Detected path traversal attempt

Hi All,

I bumped my Django project to the latest 3.2.4 release from 3.2. All tests passed (OK, I didn’t have a test for this endpoint) and I was happy as can be. I was testing my application which is a JS SPA + Django API and I noticed that media upload to AWS which goes via my Django backend were suddenly getting 400 responses. Specifically, I was getting:

SuspiciousFileOperation: Detected path traversal attempt in '/case_data/dogs/01000/dogs_1000_1622660073141.jpeg'

I took a look at the 3.2.4 release notes and saw that there was a patch for a potential directory traversal vulnerability and I started to suspect it might be hitting my code.

The exception is raised when I return instance in the below code:

    def create_media(self, file, filename, user, short_name=None, annotation=None):
        try:
            instance = self.model(
                file=filename, uploaded_by=user, short_name=short_name, annotation=annotation
            )
            instance.save()
            instance.file.save(filename, file)
            return instance
        except IntegrityError:

And this is my model which inherits from a Meta class, also below.

class Media(models.Model):
    secure_file = models.CharField(null=True, max_length=512)
    key = models.CharField(max_length=512, null=True)
    uploaded_by = models.ForeignKey(
        settings.AUTH_USER_MODEL, on_delete=models.CASCADE, null=True, blank=True
    )

    class Meta:
        abstract = True


class Image(Media):
    id = models.UUIDField(primary_key=True, default=uuid.uuid4)
    file = models.FileField(upload_to="images", unique=True)
    annotation = models.CharField(max_length=1024, null=True, blank=True)
    short_name = models.CharField(max_length=256, null=True, blank=True)
    url = models.URLField(null=True, blank=True)
    created_by = models.ForeignKey(settings.AUTH_USER_MODEL, on_delete=models.SET_NULL, null=True, blank=True, related_name="images")

So far, at least to my eye, everything looks very standard.

The Django source code which is causing the problem is:

# My own comment: This is from 3.2.4 source, 
# but it is identical to 3.2 source as best I can tell

    if allow_relative_path:
        # Use PurePosixPath() because this branch is checked only in
        # FileField.generate_filename() where all file paths are expected to be
        # Unix style (with forward slashes).
        path = pathlib.PurePosixPath(name)
        if path.is_absolute() or '..' in path.parts:
            raise SuspiciousFileOperation(
                "Detected path traversal attempt in '%s'" % name
            )

What is perhaps most interesting is that my file path which begins with a / is true when executing path.is_absolute()

>>> import pathlib
>>> name = '/case_data/snakes/01001/snakes_1001_1622659350659.jpeg'
>>> path = pathlib.PurePosixPath(name)
>>> path.is_absolute()
True
>>> '..' in path.parts
False

>>> name = 'snakes_1001_1622659350659.jpeg'
>>> path = pathlib.PurePosixPath(name)
>>> path.is_absolute()
False
>>> name = 'case_data/snakes/01001/snakes_1001_1622659350659.jpeg'
>>> path = pathlib.PurePosixPath(name)
>>> path.is_absolute()
False
>>>

And finally, my question:

Given my file path string, '/case_data/dogs/01000/dogs_1000_1622660073141.jpeg' is the same when I run 3.2 and 3.2.4 code and I can reliably reproduce the above described behaviour, why am I not getting the SuspiciousFileOperation in 3.2 but I am in 3.2.4?

I considered opening a ticket, but I’m loathe to do so without checking here first lest I’m missing something obvious.

As always, thank you for your help.

Cheers,

Conor

Hi Conor,

3.2.3 should have fixed a regression here (Django 3.2.3 release notes | Django documentation | Django) – do you have a traceback to share, I wonder where exactly this is raised…

That said, the error is somewhat expected, when you call instance.file.save the filename must be relative to MEDIA_ROOT/upload_to. 3.2.0 was more lenient here which we started fixing with 3.2.1.

What is the final upload url you want it to have?

Cheers,
Florian

Hi Florian,

Thanks for helping out. I see that my path is not relative to my upload_to so based on what you write, that makes sense. The upload to is left over from earlier iterations of my code. There is a requirement that images be uploaded to AWS in a human friendly directory structure. In my code example below I demonstrate where the file path comes from.

The stacktrace is:


File "/usr/local/lib/python3.8/site-packages/django/core/handlers/base.py" line 181 in _get_response
response = wrapped_callback(request, *callback_args, **callback_kwargs)

File "/usr/local/lib/python3.8/site-packages/django/views/decorators/csrf.py" line 54 in wrapped_view
return view_func(*args, **kwargs)

File "/usr/local/lib/python3.8/site-packages/django/views/generic/base.py" line 70 in view
return self.dispatch(request, *args, **kwargs)

File "/usr/local/lib/python3.8/site-packages/rest_framework/views.py" line 509 in dispatch
response = self.handle_exception(exc)

File "/usr/local/lib/python3.8/site-packages/rest_framework/views.py" line 469 in handle_exception
self.raise_uncaught_exception(exc)

File "/usr/local/lib/python3.8/site-packages/rest_framework/views.py" line 480 in raise_uncaught_exception
raise exc

File "/usr/local/lib/python3.8/site-packages/rest_framework/views.py" line 506 in dispatch
response = handler(request, *args, **kwargs)

File "/code/media/views.py", line 106 in post [args] [locals]
instance = self.create_media(

File "/code/media/views.py", line 142 in create_media [args] [locals]
instance.file.save(filename, file)
- 3 non-project frames

File "/usr/local/lib/python3.8/site-packages/django/db/models/fields/files.py" line 88 in save
name = self.field.generate_filename(self.instance, name)

File "/usr/local/lib/python3.8/site-packages/django/db/models/fields/files.py" line 321 in generate_filename
filename = validate_file_name(filename, allow_relative_path=True)

File "/usr/local/lib/python3.8/site-packages/django/core/files/utils.py" line 18 in validate_file_name [args] [locals]
raise SuspiciousFileOperation(
SuspiciousFileOperation: Detected path traversal attempt in '/case_data/dogs/01000/dogs_1000_1622660073141.jpeg'

The final URL is:

https://my-bucket.s3.amazonaws.com/case_data/dogs/01000/dogs_1000_1622660073141.jpeg

I’ve added some more context to my code below.

class MediaUploadSerializer(serializers.Serializer):
    filename = serializers.CharField(max_length=64, min_length=1, required=True)
    short_name = serializers.CharField(max_length=64, min_length=1, required=True)
    annotation = serializers.CharField(max_length=1024, min_length=1, required=False)
    file = serializers.FileField(required=True)
    case = serializers.UUIDField(required=True)

class MediaCreatorView(views.APIView):
    serializer_class = serializers.MediaUploadSerializer
    model = # in this instance it an Image as in show in my first post

   def post(self, request, *args, **kwargs):
      # ...
      file = request.FILES["file"]  # FileField
      filename = serializer.validated_data.get("filename", None)  # CharField
      # ..
      # the below returns the path of where the file is stored on AWS.
      # this results in the string:
      # "/case_data/dogs/01000/dogs_1000_1622660073141.jpeg"

      filename = utils.aws_filename_key(group, case) + filename

      try:
            instance = self.create_media(
                file, filename, request.user, short_name=short_name, annotation=annotation
            )
      # ...

    def create_media(self, file, filename, user, short_name=None, annotation=None):
        try:
            instance = self.model(
                file=filename, uploaded_by=user, short_name=short_name, annotation=annotation
            )
            instance.save()
            instance.file.save(filename, file)
            return instance
        except IntegrityError:

If I have understood the problem correctly, I will need to have my file path relative to upload_to=images. Therefore I should have:

images/case_data/dogs/01000/dogs_1000_1622660073141.jpeg

instead of

/case_data/dogs/01000/dogs_1000_1622660073141.jpeg

As a test, I modified my aws key generating code to this:

filename = "images" + utils.aws_filename_key(group, case) + filename
# filename now equals: "images/case_data/dogs/01000/dogs_1000_1622660073141.jpeg"

With this small hack, I can now upload my images using 3.2.4.

Have I understood you correctly or am I still off in la la land?

Cheers,

Conor

Since you have already set upload_to on the FileField to images it should be enough to remove the initial slash, Django will then join that internally: django/files.py at f10c52afabac25f2c10aca26d32dbe7e0e46082e · django/django · GitHub (and would be the prefered way).

Nope, that sounds good, but see my comment above about no repeating images.

Cheers,
Florian

Thanks a million, Florian, you’ve helped me get it sorted.

I’ve modified all my media models to upload_to="case_data" and my utils.aws_filename_key() method no longer prepends case_data to the return data which is which is now also without the initial /.

Tested these changes locally and all looks to be working well. Now, I’d better go and add some more tests.

Again, thank you, you’ve been a big help.

Cheers,

Conor