Opened 3 years ago

Closed 3 years ago

#33287 closed Cleanup/optimization (fixed)

eval() in geojson serializer (hardening)

Reported by: David Wyde Owned by: Maxim Piskunov
Component: GIS Version: 3.2
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

In Django 3.2.9, there is a call to eval() in django/contrib/gis/serializers/geojson.py:

  6 class Serializer(JSONSerializer):
 38     def get_dump_object(self, obj):
 53             data["geometry"] = eval(self._geometry.geojson)

Given an app/models.py of

from django.db import models

class Model(models.Model):
    value = models.Field()

and a demo.py of

from django.core import serializers
from app.models import Model

class Geo:
    def __init__(self):
        self.srid = 4326
        self.geojson = '__import__("os").system("ls -l")'

geo = Geo()
m = Model(value=geo)
serializers.serialize('geojson', [m], geometry_field='value')

python3 manage.py shell <demo.py runs ls -l.

The above code requires that django.contrib.gis is in INSTALLED_APPS
and apt install gdal-bin was run on Ubuntu 20.04.

I've been told by the Django security team that this won't be exploited through normal use of the geojson serializer, so this is a hardening issue that can be tracked in Trac.

Change History (8)

comment:1 by Michael Manfre, 3 years ago

Triage Stage: UnreviewedAccepted

comment:2 by Simon Charette, 3 years ago

I've confirmed that switching eval to json.loads addresses the issue and passes the test suite. David, would you like to submit a PR with this change?

comment:3 by Mariusz Felisiak, 3 years ago

Type: UncategorizedCleanup/optimization

comment:4 by Maxim Piskunov, 3 years ago

Has patch: set
Owner: changed from nobody to Maxim Piskunov
Status: newassigned

comment:5 by David Wyde, 3 years ago

Owner: changed from Maxim Piskunov to David Wyde

It looks like there's https://github.com/django/django/pull/15088, but I submitted https://github.com/django/django/pull/15089.

I'm getting some errors and failures when I run the python3 runtests.py gis_tests --settings settings where settings.py sets a MySQL GIS backend, but the eval/json.loads change doesn't affect that. I filed https://code.djangoproject.com/ticket/33292 for that.

comment:6 by Mariusz Felisiak, 3 years ago

Owner: changed from David Wyde to Maxim Piskunov

comment:7 by Mariusz Felisiak, 3 years ago

Triage Stage: AcceptedReady for checkin

comment:8 by Mariusz Felisiak <felisiak.mariusz@…>, 3 years ago

Resolution: fixed
Status: assignedclosed

In 12fe322:

Fixed #33287 -- Made GeoJSON serializer use json.loads() instead of eval().

Thanks David Wyde for the report.

Note: See TracTickets for help on using tickets.
Back to Top