ModelForm shouldn't fallback to forms.CharField for RasterField

[Jacob Walls]

Recent security reports around raster objects pointed out that ModelForm will fallback to forms.CharField by default, e.g. in the admin.

After the effort we went to in f1949c1f9758947ade984c895ff16bef46f56520 to advise folks to write custom validation when accepting raster definitions from untrusted user input, we could enforce this by refusing to let ModelForm fallback to CharField.

My first idea is to simply register a RasterField that raises. (Or raises after a deprecation, but we can also consider this a "security feature" that ModelForm no longer falls back to something clearly inadequate and just get it in for 6.2? The API stability policy contemplates security as an exception.)

Sketch:

---

django/contrib/gis/db/models/fields.py

@@ -423 +423 @@
     """
 
     description = _("Raster Field")
+    form_class = forms.RasterField
     geom_type = "RASTER"
     geography = False
 
@@ -452 +453 @@
         # of the raster attribute.
         setattr(cls, self.attname, SpatialProxy(gdal.GDALRaster, self))
 
+    def formfield(self, **kwargs):
+        return super().formfield(
+            **{
+                "form_class": self.form_class,
+                **kwargs,
+            }
+        )
+
     def get_transform(self, name):
         from django.contrib.gis.db.models.lookups import RasterBandTransform
 

django/contrib/gis/forms/__init__.py

@@ -9 +9 @@
     MultiPolygonField,
     PointField,
     PolygonField,
+    RasterField,
 )
 from .widgets import BaseGeometryWidget, OpenLayersWidget, OSMWidget  # NOQA

django/contrib/gis/forms/fields.py

@@ -7 +7 @@
 from .widgets import OpenLayersWidget
 
 
+class RasterField(forms.Field):
+    geom_type = "RASTER"
+
+    def __init__(self, *args, **kwargs):
+        raise NotImplementedError("Security: do your own validation...!")
+
+
 class GeometryField(forms.Field):
     """
     This is the basic form field for a Geometry. Any textual input that is

tests/gis_tests/rasterapp/test_rasterfield.py

@@ -1 +1 @@
 import json
 from unittest import mock
 
+from django.contrib import admin
 from django.contrib.gis.db.models.fields import BaseSpatialField
 from django.contrib.gis.db.models.functions import Distance
 from django.contrib.gis.db.models.lookups import (
@@ -14 +15 @@
 from django.contrib.gis.shortcuts import numpy
 from django.db import connection
 from django.db.models import F, Func, Q
-from django.test import TransactionTestCase, skipUnlessDBFeature
+from django.test import TestCase, TransactionTestCase, skipUnlessDBFeature
 from django.test.utils import CaptureQueriesContext
 
 from ..data.rasters.textrasters import JSON_RASTER
 from .models import RasterModel, RasterRelatedModel
 
 
+site = admin.AdminSite(name="rasterapp_modeladmin")
+site.register(RasterModel, admin.ModelAdmin)
+
+
 @skipUnlessDBFeature("supports_raster")
 class RasterFieldTest(TransactionTestCase):
     available_apps = ["gis_tests.rasterapp"]
@@ -502 +507 @@
         # It's easier to check the indexes in the generated SQL than to write
         # tests that cover all index combinations.
         self.assertRegex(queries[-1]["sql"], r"WHERE ST_Contains\([^)]*, 1, [^)]*, 1\)")
+
+
+@skipUnlessDBFeature("supports_raster")
+class RasterFieldAdminTest(TestCase):
+    def test_form_raises(self):
+        geoadmin = site.get_model_admin(RasterModel)
+        with self.assertRaises(NotImplementedError):
+            geoadmin.get_changelist_form(None)()

tests/gis_tests/test_geoforms.py

@@ -12 +12 @@
 from .data.rasters.textrasters import JSON_RASTER
 
 
+class RasterFieldTest(SimpleTestCase):
+    ...
+
+
 class GeometryFieldTest(SimpleTestCase):
     def test_init(self):
         "Testing GeometryField initialization with defaults."