From 72d8819eef87adbcbbedc80084a2e6652447619f Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Wed, 15 Jul 2026 15:39:27 -0400 Subject: [PATCH] [5.2.x] Fixed CVE-2026-15830 -- Mitigated potential DoS via nested geometry collections. Since deeply nested geometry collections can lead to fatal errors in GEOS, a new `max_geom_collections` argument on geometry model and form fields, passed down to `GEOSGeometry` itself, allows limiting either depth (WKT) or total number (WKB) before reaching GEOS. Thanks Andrew MacPherson and kimchunbok_ for the reports, and Natalia Bidart, Simon Charette, and Sarah Boyce for reviews. Backport of d2e59b77fe18de318a8272c2a7bbc798d84d1d0d from main. CVE: CVE-2026-15830 Upstream-Status: Backport [https://github.com/django/django/commit/ba80833fa656dd09660b97c4429331067db1b080] Backport Changes: - Dropped the docs/releases/5.2.17.txt file as current version for Scarthgap is 5.0.14 (cherry picked from commit ba80833fa656dd09660b97c4429331067db1b080) Signed-off-by: Darsh Kelaiya --- .../contrib/gis/db/backends/mysql/features.py | 14 ++ .../gis/db/backends/mysql/operations.py | 4 +- .../gis/db/backends/oracle/features.py | 4 + .../gis/db/backends/oracle/operations.py | 5 +- .../gis/db/backends/postgis/operations.py | 9 +- .../gis/db/backends/spatialite/operations.py | 5 +- django/contrib/gis/db/models/fields.py | 18 +- django/contrib/gis/db/models/proxy.py | 7 +- django/contrib/gis/forms/fields.py | 16 +- django/contrib/gis/forms/widgets.py | 4 +- django/contrib/gis/geos/geometry.py | 31 ++- django/contrib/gis/geos/prototypes/io.py | 161 ++++++++++++++-- docs/ref/contrib/gis/forms-api.txt | 24 +++ docs/ref/contrib/gis/geos.txt | 8 +- docs/ref/contrib/gis/model-api.txt | 14 ++ tests/gis_tests/geoapp/models.py | 4 + tests/gis_tests/geoapp/tests.py | 59 +++++- tests/gis_tests/geos_tests/test_geos_limit.py | 176 ++++++++++++++++++ tests/gis_tests/rasterapp/test_rasterfield.py | 16 ++ tests/gis_tests/test_fields.py | 33 ++++ tests/gis_tests/test_geoforms.py | 50 +++++ 21 files changed, 629 insertions(+), 33 deletions(-) create mode 100644 tests/gis_tests/geos_tests/test_geos_limit.py diff --git a/django/contrib/gis/db/backends/mysql/features.py b/django/contrib/gis/db/backends/mysql/features.py index cd99420374..78e58617ec 100644 --- a/django/contrib/gis/db/backends/mysql/features.py +++ b/django/contrib/gis/db/backends/mysql/features.py @@ -19,3 +19,17 @@ class DatabaseFeatures(BaseSpatialFeatures, MySQLDatabaseFeatures): def supports_geometry_field_unique_index(self): # Not supported in MySQL since https://dev.mysql.com/worklog/task/?id=11808 return self.connection.mysql_is_mariadb + + @cached_property + def django_test_skips(self): + skips = super().django_test_skips + if self.connection.mysql_is_mariadb: + skips.update( + { + "MariaDB doesn't support nested geometry collections.": { + "gis_tests.geoapp.tests.SaveLoadTests." + "test_geometrycollectionfield_default_max_ignored_on_read", + }, + } + ) + return skips diff --git a/django/contrib/gis/db/backends/mysql/operations.py b/django/contrib/gis/db/backends/mysql/operations.py index 886db605cd..4d6103dc5b 100644 --- a/django/contrib/gis/db/backends/mysql/operations.py +++ b/django/contrib/gis/db/backends/mysql/operations.py @@ -122,7 +122,9 @@ class MySQLOperations(BaseSpatialOperations, DatabaseOperations): def converter(value, expression, connection): if value is not None: - geom = GEOSGeometryBase(read(memoryview(value)), geom_class) + geom = GEOSGeometryBase( + read(memoryview(value), max_geom_collections=None), geom_class + ) if srid: geom.srid = srid return geom diff --git a/django/contrib/gis/db/backends/oracle/features.py b/django/contrib/gis/db/backends/oracle/features.py index f346d93573..16279b8a1e 100644 --- a/django/contrib/gis/db/backends/oracle/features.py +++ b/django/contrib/gis/db/backends/oracle/features.py @@ -23,6 +23,10 @@ class DatabaseFeatures(BaseSpatialFeatures, OracleDatabaseFeatures): "gis_tests.gis_migrations.test_operations.OperationTests." "test_add_check_constraint", }, + "Oracle doesn't support nested geometry collections.": { + "gis_tests.geoapp.tests.SaveLoadTests." + "test_geometrycollectionfield_default_max_ignored_on_read", + }, } ) return skips diff --git a/django/contrib/gis/db/backends/oracle/operations.py b/django/contrib/gis/db/backends/oracle/operations.py index eb86dc39de..59366eadc0 100644 --- a/django/contrib/gis/db/backends/oracle/operations.py +++ b/django/contrib/gis/db/backends/oracle/operations.py @@ -236,7 +236,10 @@ class OracleOperations(BaseSpatialOperations, DatabaseOperations): def converter(value, expression, connection): if value is not None: - geom = GEOSGeometryBase(read(memoryview(value.read())), geom_class) + geom = GEOSGeometryBase( + read(memoryview(value.read()), max_geom_collections=None), + geom_class, + ) if srid: geom.srid = srid return geom diff --git a/django/contrib/gis/db/backends/postgis/operations.py b/django/contrib/gis/db/backends/postgis/operations.py index b68db377f8..381e408bc7 100644 --- a/django/contrib/gis/db/backends/postgis/operations.py +++ b/django/contrib/gis/db/backends/postgis/operations.py @@ -413,9 +413,12 @@ class PostGISOperations(BaseSpatialOperations, DatabaseOperations): geom_class = expression.output_field.geom_class def converter(value, expression, connection): - if isinstance(value, str): # Coming from hex strings. - value = value.encode("ascii") - return None if value is None else GEOSGeometryBase(read(value), geom_class) + if value is not None: + if isinstance(value, str): # Coming from hex strings. + value = value.encode("ascii") + return GEOSGeometryBase( + read(value, max_geom_collections=None), geom_class + ) return converter diff --git a/django/contrib/gis/db/backends/spatialite/operations.py b/django/contrib/gis/db/backends/spatialite/operations.py index d39f7a9e0d..721d9fa047 100644 --- a/django/contrib/gis/db/backends/spatialite/operations.py +++ b/django/contrib/gis/db/backends/spatialite/operations.py @@ -223,6 +223,9 @@ class SpatiaLiteOperations(BaseSpatialOperations, DatabaseOperations): read = wkb_r().read def converter(value, expression, connection): - return None if value is None else GEOSGeometryBase(read(value), geom_class) + if value is not None: + return GEOSGeometryBase( + read(value, max_geom_collections=None), geom_class + ) return converter diff --git a/django/contrib/gis/db/models/fields.py b/django/contrib/gis/db/models/fields.py index 15d9ae3c57..d3f9b1b2ff 100644 --- a/django/contrib/gis/db/models/fields.py +++ b/django/contrib/gis/db/models/fields.py @@ -17,6 +17,7 @@ from django.contrib.gis.geos import ( Point, Polygon, ) +from django.contrib.gis.geos.prototypes.io import MAX_GEOM_COLLECTIONS from django.core.exceptions import ImproperlyConfigured from django.db.models import Field from django.utils.translation import gettext_lazy as _ @@ -215,8 +216,11 @@ class BaseSpatialField(Field): if raster: obj = raster elif is_candidate: + max_geom_collections = getattr( + self, "max_geom_collections", MAX_GEOM_COLLECTIONS + ) try: - obj = GEOSGeometry(obj) + obj = GEOSGeometry(obj, max_geom_collections=max_geom_collections) except (TypeError, ValueError) as err: if isinstance(obj, str) and obj.startswith(VSI_FILESYSTEM_PREFIX): raise blocked_err @@ -260,6 +264,7 @@ class GeometryField(BaseSpatialField): *, extent=(-180.0, -90.0, 180.0, 90.0), tolerance=0.05, + max_geom_collections=MAX_GEOM_COLLECTIONS, **kwargs, ): """ @@ -278,6 +283,10 @@ class GeometryField(BaseSpatialField): tolerance: Define the tolerance, in meters, to use for the geometry field entry in the `USER_SDO_GEOM_METADATA` table. Defaults to 0.05. + + max_geom_collections: + The maximum number of geometry collections accepted before parsing is + refused, forwarded to the form field. """ # Setting the dimension of the geometry field. self.dim = dim @@ -290,6 +299,10 @@ class GeometryField(BaseSpatialField): self._extent = extent self._tolerance = tolerance + # Limit on nested/total geometry collections, forwarded to the form + # field to guard against crashes in GEOS from deeply nested input. + self.max_geom_collections = max_geom_collections + super().__init__(verbose_name=verbose_name, **kwargs) def deconstruct(self): @@ -303,6 +316,8 @@ class GeometryField(BaseSpatialField): kwargs["extent"] = self._extent if self._tolerance != 0.05: kwargs["tolerance"] = self._tolerance + if self.max_geom_collections != MAX_GEOM_COLLECTIONS: + kwargs["max_geom_collections"] = self.max_geom_collections return name, path, args, kwargs def contribute_to_class(self, cls, name, **kwargs): @@ -320,6 +335,7 @@ class GeometryField(BaseSpatialField): "form_class": self.form_class, "geom_type": self.geom_type, "srid": self.srid, + "max_geom_collections": self.max_geom_collections, **kwargs, } if self.dim > 2 and not getattr( diff --git a/django/contrib/gis/db/models/proxy.py b/django/contrib/gis/db/models/proxy.py index b415e147fc..e842ad0f22 100644 --- a/django/contrib/gis/db/models/proxy.py +++ b/django/contrib/gis/db/models/proxy.py @@ -43,7 +43,12 @@ class SpatialProxy(DeferredAttribute): else: # Otherwise, a geometry or raster object is built using the field's # contents, and the model's corresponding attribute is set. - geo_obj = self._load_func(geo_value) + try: + max_geoms = self.field.max_geom_collections + except AttributeError: + geo_obj = self._load_func(geo_value) + else: + geo_obj = self._load_func(geo_value, max_geom_collections=max_geoms) setattr(instance, self.field.attname, geo_obj) return geo_obj diff --git a/django/contrib/gis/forms/fields.py b/django/contrib/gis/forms/fields.py index 1fd31530c1..7835b22ced 100644 --- a/django/contrib/gis/forms/fields.py +++ b/django/contrib/gis/forms/fields.py @@ -1,6 +1,7 @@ from django import forms from django.contrib.gis.gdal import GDALException from django.contrib.gis.geos import GEOSException, GEOSGeometry +from django.contrib.gis.geos.prototypes.io import MAX_GEOM_COLLECTIONS from django.core.exceptions import ValidationError from django.utils.translation import gettext_lazy as _ @@ -16,6 +17,7 @@ class GeometryField(forms.Field): widget = OpenLayersWidget geom_type = "GEOMETRY" + max_geom_collections = MAX_GEOM_COLLECTIONS default_error_messages = { "required": _("No geometry value provided."), @@ -27,12 +29,20 @@ class GeometryField(forms.Field): ), } - def __init__(self, *, srid=None, geom_type=None, **kwargs): + def __init__( + self, *, srid=None, geom_type=None, max_geom_collections=None, **kwargs + ): self.srid = srid if geom_type is not None: self.geom_type = geom_type + if max_geom_collections is not None: + self.max_geom_collections = max_geom_collections super().__init__(**kwargs) self.widget.attrs["geom_type"] = self.geom_type + # Propagate the limit to the (per-field) widget instance, which does + # the actual parsing. Custom widgets that override deserialize() and + # ignore this attribute still get the default limit via GEOSGeometry. + self.widget.max_geom_collections = self.max_geom_collections def to_python(self, value): """Transform the value to a Geometry object.""" @@ -47,7 +57,9 @@ class GeometryField(forms.Field): value = None else: try: - value = GEOSGeometry(value) + value = GEOSGeometry( + value, max_geom_collections=self.max_geom_collections + ) except (GEOSException, ValueError, TypeError): value = None if value is None: diff --git a/django/contrib/gis/forms/widgets.py b/django/contrib/gis/forms/widgets.py index 49ca48794b..6c9fac774c 100644 --- a/django/contrib/gis/forms/widgets.py +++ b/django/contrib/gis/forms/widgets.py @@ -5,6 +5,7 @@ from django.conf import settings from django.contrib.gis import gdal from django.contrib.gis.geometry import json_regex from django.contrib.gis.geos import GEOSException, GEOSGeometry +from django.contrib.gis.geos.prototypes.io import MAX_GEOM_COLLECTIONS from django.forms.widgets import Widget from django.utils import translation from django.utils.deprecation import RemovedInDjango51Warning @@ -23,6 +24,7 @@ class BaseGeometryWidget(Widget): map_width = 600 # RemovedInDjango51Warning map_height = 400 # RemovedInDjango51Warning display_raw = False + max_geom_collections = MAX_GEOM_COLLECTIONS supports_3d = False template_name = "" # set on subclasses @@ -50,7 +52,7 @@ class BaseGeometryWidget(Widget): def deserialize(self, value): try: - return GEOSGeometry(value) + return GEOSGeometry(value, max_geom_collections=self.max_geom_collections) except (GEOSException, ValueError, TypeError) as err: logger.error("Error creating geometry from value '%s' (%s)", value, err) return None diff --git a/django/contrib/gis/geos/geometry.py b/django/contrib/gis/geos/geometry.py index 8bbe2c264a..a505f03ee7 100644 --- a/django/contrib/gis/geos/geometry.py +++ b/django/contrib/gis/geos/geometry.py @@ -15,7 +15,14 @@ from django.contrib.gis.geos.error import GEOSException from django.contrib.gis.geos.libgeos import GEOM_PTR, geos_version_tuple from django.contrib.gis.geos.mutable_list import ListMixin from django.contrib.gis.geos.prepared import PreparedGeometry -from django.contrib.gis.geos.prototypes.io import ewkb_w, wkb_r, wkb_w, wkt_r, wkt_w +from django.contrib.gis.geos.prototypes.io import ( + MAX_GEOM_COLLECTIONS, + ewkb_w, + wkb_r, + wkb_w, + wkt_r, + wkt_w, +) from django.utils.deconstruct import deconstructible from django.utils.encoding import force_bytes, force_str @@ -113,8 +120,8 @@ class GEOSGeometryBase(GEOSBase): self.srid = srid @classmethod - def _from_wkb(cls, wkb): - return wkb_r().read(wkb) + def _from_wkb(cls, wkb, max_geom_collections=MAX_GEOM_COLLECTIONS): + return wkb_r().read(wkb, max_geom_collections) @staticmethod def from_ewkt(ewkt): @@ -134,8 +141,8 @@ class GEOSGeometryBase(GEOSBase): return GEOSGeometry(GEOSGeometry._from_wkt(wkt), srid=srid) @staticmethod - def _from_wkt(wkt): - return wkt_r().read(wkt) + def _from_wkt(wkt, max_geom_collections=MAX_GEOM_COLLECTIONS): + return wkt_r().read(wkt, max_geom_collections) @classmethod def from_gml(cls, gml_string): @@ -720,7 +727,9 @@ class LinearGeometryMixin: class GEOSGeometry(GEOSGeometryBase, ListMixin): "A class that, generally, encapsulates a GEOS geometry." - def __init__(self, geo_input, srid=None): + def __init__( + self, geo_input, srid=None, *, max_geom_collections=MAX_GEOM_COLLECTIONS + ): """ The base constructor for GEOS geometry objects. It may take the following inputs: @@ -734,6 +743,10 @@ class GEOSGeometry(GEOSGeometryBase, ListMixin): The `srid` keyword specifies the Source Reference Identifier (SRID) number for this Geometry. If not provided, it defaults to None. + + The `max_geom_collections` keyword limits how many nested (WKT) or + total (WKB) geometry collections the input may contain before parsing + is refused, guarding against segfaults from deeply nested input. """ input_srid = None if isinstance(geo_input, bytes): @@ -744,10 +757,10 @@ class GEOSGeometry(GEOSGeometryBase, ListMixin): # Handle WKT input. if wkt_m["srid"]: input_srid = int(wkt_m["srid"]) - g = self._from_wkt(force_bytes(wkt_m["wkt"])) + g = self._from_wkt(force_bytes(wkt_m["wkt"]), max_geom_collections) elif hex_regex.match(geo_input): # Handle HEXEWKB input. - g = wkb_r().read(force_bytes(geo_input)) + g = wkb_r().read(force_bytes(geo_input), max_geom_collections) elif json_regex.match(geo_input): # Handle GeoJSON input. ogr = gdal.OGRGeometry.from_json(geo_input) @@ -760,7 +773,7 @@ class GEOSGeometry(GEOSGeometryBase, ListMixin): g = geo_input elif isinstance(geo_input, memoryview): # When the input is a memoryview (WKB). - g = wkb_r().read(geo_input) + g = wkb_r().read(geo_input, max_geom_collections) elif isinstance(geo_input, GEOSGeometry): g = capi.geom_clone(geo_input.ptr) else: diff --git a/django/contrib/gis/geos/prototypes/io.py b/django/contrib/gis/geos/prototypes/io.py index efe9ec159f..c57145f85c 100644 --- a/django/contrib/gis/geos/prototypes/io.py +++ b/django/contrib/gis/geos/prototypes/io.py @@ -1,3 +1,4 @@ +import re import threading from ctypes import POINTER, Structure, byref, c_byte, c_char_p, c_int, c_size_t @@ -15,6 +16,7 @@ from django.contrib.gis.geos.prototypes.errcheck import ( from django.contrib.gis.geos.prototypes.geom import c_uchar_p, geos_char_p from django.utils.encoding import force_bytes from django.utils.functional import SimpleLazyObject +from django.utils.regex_helper import _lazy_re_compile # ### The WKB/WKT Reader/Writer structures and pointers ### @@ -145,6 +147,58 @@ class IOBase(GEOSBase): # ### Base WKB/WKT Reading and Writing objects ### +# Sits just under PostGIS's effective ceiling: liblwgeom's LW_PARSER_MAX_DEPTH +# is 200 and counts the leaf geometry, so PostGIS rejects at 199 nested +# collections. 198 keeps Django's guard below that (and far below the GEOS +# segfault threshold) so it rejects before any backend supporting nested +# geometries does. (Oracle and MariaDB don't support nesting.) +MAX_GEOM_COLLECTIONS = 198 + +# GEOS accepts any amount of whitespace around the optional dimension marker, +# so the separators must be \s*, not \s? or \s+. The root variants also allow +# leading whitespace, which GEOS skips before the geometry type. +_WKT_COLLECTION_START_RE = _lazy_re_compile( + r"\bGEOMETRYCOLLECTION(?:\s*(?:ZM|Z|M))?\s*\(", + re.IGNORECASE, +) +_WKT_COLLECTION_START_BYTES_RE = _lazy_re_compile( + rb"\bGEOMETRYCOLLECTION(?:\s*(?:ZM|Z|M))?\s*\(", + re.IGNORECASE, +) +_WKT_COLLECTION_ROOT_RE = _lazy_re_compile( + r"\s*\bGEOMETRYCOLLECTION(?:\s*(?:ZM|Z|M))?\s*\(", + re.IGNORECASE, +) +_WKT_COLLECTION_ROOT_BYTES_RE = _lazy_re_compile( + rb"\s*\bGEOMETRYCOLLECTION(?:\s*(?:ZM|Z|M))?\s*\(", + re.IGNORECASE, +) + + +def _build_collection_header_re(): + """GEOS normalizes WKB types using: (type_code & 0xFFFF) % 1000 + + Therefore, every low 16-bit value congruent to 7 modulo 1000 is interpreted + as a GeometryCollection. Upper 16 bits may contain arbitrary EWKB flags. + """ + low_types = range(7, 0x10000, 1000) + little_endian_types = b"|".join( + re.escape(type_code.to_bytes(2, "little")) for type_code in low_types + ) + big_endian_types = b"|".join( + re.escape(type_code.to_bytes(2, "big")) for type_code in low_types + ) + return _lazy_re_compile( + rb"(?=(" + rb"\x01(?:" + little_endian_types + rb")[\x00-\xff]{2}" + rb"|" + rb"\x00[\x00-\xff]{2}(?:" + big_endian_types + rb")" + rb"))" + ) + + +_COLLECTION_HEADER_RE = _build_collection_header_re() + # Non-public WKB/WKT reader classes for internal use because # their `read` methods return _pointers_ instead of GEOSGeometry @@ -154,9 +208,48 @@ class _WKTReader(IOBase): ptr_type = WKT_READ_PTR destructor = wkt_reader_destroy - def read(self, wkt): + def limit(self, wkt, max_geom_collections): + if max_geom_collections is None: + return + if isinstance(wkt, str): + pattern = _WKT_COLLECTION_START_RE + root_pattern = _WKT_COLLECTION_ROOT_RE + open_paren = "(" + close_paren = ")" + else: + pattern = _WKT_COLLECTION_START_BYTES_RE + root_pattern = _WKT_COLLECTION_ROOT_BYTES_RE + open_paren = ord("(") + close_paren = ord(")") + if root_pattern.match(wkt) is None: + # Fast path: If the beginning does not match GEOMETRYCOLLECTION(, + # then GEOS rejects early (no need to limit): + # GEOS_ERROR: countered : 'GEOMETRYCOLLECTION' + return + collection_starts = {match.end() - 1 for match in pattern.finditer(wkt)} + # Nesting depth can't exceed the total number of collections, so if the + # total is already within the limit, there is nothing to walk. + if len(collection_starts) <= max_geom_collections: + return + collection_depth = 0 + parentheses = [] + for index, char in enumerate(wkt): + if char == open_paren: + is_collection = index in collection_starts + parentheses.append(is_collection) + if is_collection: + collection_depth += 1 + elif char == close_paren and parentheses: + if parentheses.pop(): + collection_depth -= 1 + if collection_depth > max_geom_collections: + msg = "WKT contains too many possible GeometryCollections." + raise ValueError(msg) + + def read(self, wkt, max_geom_collections=MAX_GEOM_COLLECTIONS): if not isinstance(wkt, (bytes, str)): - raise TypeError + raise TypeError(f"'wkt' must be bytes or str (got {wkt!r} instead).") + self.limit(wkt, max_geom_collections) return wkt_reader_read(self.ptr, force_bytes(wkt)) @@ -165,18 +258,64 @@ class _WKBReader(IOBase): ptr_type = WKB_READ_PTR destructor = wkb_reader_destroy - def read(self, wkb): + def limit_wkb(self, wkb, max_geom_collections): + if max_geom_collections is None: + return + for count, _ in enumerate(_COLLECTION_HEADER_RE.finditer(wkb), 1): + if count > max_geom_collections: + msg = "WKB contains too many possible GeometryCollections." + raise ValueError(msg) + + def limit_hex(self, wkb, max_geom_collections): + if max_geom_collections is None: + return + + def _byteswap_uint32(value): + return ( + ((value & 0x000000FF) << 24) + | ((value & 0x0000FF00) << 8) + | ((value & 0x00FF0000) >> 8) + | ((value & 0xFF000000) >> 24) + ) + + count = 0 + for index in range(0, len(wkb) - 9, 2): + byte_order = wkb[index : index + 2] + if byte_order not in (b"00", b"01"): + continue + try: + geometry_type = int(wkb[index + 2 : index + 10], 16) + except ValueError: + continue + geometry_type = _byteswap_uint32(geometry_type) + # Match GEOS WKBReader's geometry-type normalization. + if (geometry_type & 0xFFFF) % 1000 == 7: # GeometryCollection. + count += 1 + if count > max_geom_collections: + msg = "WKB contains too many possible GeometryCollections." + raise ValueError(msg) + + def read(self, wkb, max_geom_collections=MAX_GEOM_COLLECTIONS): "Return a _pointer_ to C GEOS Geometry object from the given WKB." + limiter = self.limit_hex + reader = wkb_reader_read_hex + if isinstance(wkb, memoryview): - wkb_s = bytes(wkb) - return wkb_reader_read(self.ptr, wkb_s, len(wkb_s)) - elif isinstance(wkb, bytes): - return wkb_reader_read_hex(self.ptr, wkb, len(wkb)) + wkb = bytes(wkb) + limiter = self.limit_wkb + reader = wkb_reader_read elif isinstance(wkb, str): - wkb_s = wkb.encode() - return wkb_reader_read_hex(self.ptr, wkb_s, len(wkb_s)) - else: - raise TypeError + wkb = wkb.encode() + elif not isinstance(wkb, bytes): + raise TypeError( + f"'wkb' must be bytes, str or memoryview (got {wkb!r} instead)." + ) + + # Limit nested geometry collections. Should become unnecessary when + # GEOS 3.15.0 is the minimum supported version. See: + # https://github.com/libgeos/geos/commit/8b8b3da7a3d9fb8953ff60bc49aa0320d51ae45c + limiter(wkb, max_geom_collections) + return reader(self.ptr, wkb, len(wkb)) def default_trim_value(): diff --git a/docs/ref/contrib/gis/forms-api.txt b/docs/ref/contrib/gis/forms-api.txt index 11e1bc77f4..9c8ed88676 100644 --- a/docs/ref/contrib/gis/forms-api.txt +++ b/docs/ref/contrib/gis/forms-api.txt @@ -36,6 +36,30 @@ GeoDjango form fields take the following optional arguments. be set up depending on the field class. It matches the OpenGIS standard geometry name. +``max_geom_collections`` +------------------------ + +.. attribute:: Field.max_geom_collections + + .. versionadded:: 5.2.17 + + The maximum number of geometry collections the field accepts before + refusing to parse the input and raising a + :exc:`~django.core.exceptions.ValidationError`. This guards against crashes + in the underlying GEOS library when parsing deeply nested + ``GEOMETRYCOLLECTION`` input. It defaults to ``198``. + + The limit is applied differently depending on the input format: for + well-known text (WKT) it bounds the nesting *depth*, while for well-known + binary (WKB and hex-encoded WKB) it bounds the *total* number of geometry + collections (both breadth and depth). As a result, the same value may + accept a wide, shallow collection as WKT but reject it as WKB. For GeoJSON, + the limit is not applied at all, since GDAL parses that input type instead. + + Increase this value (or set to ``None``) only if you must accept + legitimately deep geometries, since doing so reduces protection against + fatal errors. + Form field classes ================== diff --git a/docs/ref/contrib/gis/geos.txt b/docs/ref/contrib/gis/geos.txt index 173e51979c..8acae7cde8 100644 --- a/docs/ref/contrib/gis/geos.txt +++ b/docs/ref/contrib/gis/geos.txt @@ -207,10 +207,12 @@ Geometry Objects ``GEOSGeometry`` ---------------- -.. class:: GEOSGeometry(geo_input, srid=None) +.. class:: GEOSGeometry(geo_input, srid=None, *, max_geom_collections=198) :param geo_input: Geometry input value (string or :class:`memoryview`) :param srid: spatial reference identifier + :param max_geom_collections: maximum number of nested (WKT) or total (WKB) + geometry collections accepted before parsing is refused :type srid: int This is the base class for all GEOS geometry objects. It initializes on the @@ -248,6 +250,10 @@ WKB / EWKB ``memoryview`` For the GeoJSON format, the SRID is set based on the ``crs`` member. If ``crs`` isn't provided, the SRID defaults to 4326. +.. versionchanged:: 5.2.17 + + The ``max_geom_collections`` parameter was added. + .. classmethod:: GEOSGeometry.from_gml(gml_string) Constructs a :class:`GEOSGeometry` from the given GML string. diff --git a/docs/ref/contrib/gis/model-api.txt b/docs/ref/contrib/gis/model-api.txt index 981581cbf2..35fd0ea92b 100644 --- a/docs/ref/contrib/gis/model-api.txt +++ b/docs/ref/contrib/gis/model-api.txt @@ -223,6 +223,20 @@ details. Geography support is limited to PostGIS and will force the SRID to be 4326. +``max_geom_collections`` +------------------------ + +.. attribute:: GeometryField.max_geom_collections + +.. versionadded:: 5.2.17 + +This option is forwarded to the :attr:`form field +` generated for this model +field, bounding how many geometry collections may be contained in submitted +WKB/WKT inputs before raising :exc:`ValueError`. Since spatial field +assignments are lazy, it is also checked when values are accessed, e.g. when +saving an instance, but not when read from a database. It defaults to ``198``. + .. _geography-type: Geography Type diff --git a/tests/gis_tests/geoapp/models.py b/tests/gis_tests/geoapp/models.py index 2c13c827c6..58b92d550f 100644 --- a/tests/gis_tests/geoapp/models.py +++ b/tests/gis_tests/geoapp/models.py @@ -102,3 +102,7 @@ class ManyPointModel(NamedModel): point1 = models.PointField() point2 = models.PointField() point3 = models.PointField(srid=3857) + + +class GeometryCollectionModel(models.Model): + geom = models.GeometryCollectionField(max_geom_collections=5) diff --git a/tests/gis_tests/geoapp/tests.py b/tests/gis_tests/geoapp/tests.py index 6be13d4907..ae3470a899 100644 --- a/tests/gis_tests/geoapp/tests.py +++ b/tests/gis_tests/geoapp/tests.py @@ -1,6 +1,7 @@ import json from io import StringIO from pathlib import Path +from unittest import mock from django.contrib.gis import gdal from django.contrib.gis.db.models import Extent, MakeLine, Union, functions @@ -21,7 +22,7 @@ from django.core.files.temp import NamedTemporaryFile from django.core.management import call_command from django.db import DatabaseError, NotSupportedError, connection from django.db.models import F, OuterRef, Subquery -from django.test import TestCase, skipUnlessDBFeature +from django.test import SimpleTestCase, TestCase, skipUnlessDBFeature from django.test.utils import CaptureQueriesContext from ..data.rasters.textrasters import JSON_RASTER @@ -30,6 +31,7 @@ from .models import ( City, Country, Feature, + GeometryCollectionModel, MinusOneSRID, MultiFields, NonConcreteModel, @@ -273,6 +275,52 @@ class GeoModelTest(TestCase): self.assertEqual(feature.geom.srid, g.srid) +class SaveLoadTests(TestCase): + + def test_geometrycollectionfield_max(self): + geom = "POINT(0 0)" + for _ in range(6): + geom = f"GEOMETRYCOLLECTION({geom})" + msg = "WKT contains too many possible GeometryCollections." + with self.assertRaisesMessage(ValueError, msg): + GeometryCollectionModel.objects.create(geom=geom) + with self.assertRaisesMessage(ValueError, msg): + GeometryCollectionModel.objects.bulk_create( + [GeometryCollectionModel(geom=geom), GeometryCollectionModel(geom=geom)] + ) + + def test_geometrycollectionfield_default_max_ignored_on_read(self): + geom = "POINT(0 0)" + for _ in range(5): + geom = f"GEOMETRYCOLLECTION({geom})" + obj = GeometryCollectionModel.objects.create(geom=geom) + with ( + mock.patch( + "django.contrib.gis.geos.prototypes.io._WKBReader.limit_hex" + ) as hex_limit_mock, + mock.patch( + "django.contrib.gis.geos.prototypes.io._WKBReader.limit_wkb" + ) as wkb_limit_mock, + ): + obj.refresh_from_db() + limit_mock = hex_limit_mock if hex_limit_mock.call_count else wkb_limit_mock + limit_mock.assert_called_once() + max_geom_collections = limit_mock.call_args.args[1] + self.assertIsNone(max_geom_collections) + + +class ValidationTests(SimpleTestCase): + def test_geometrycollectionfield_max(self): + geom = "POINT(0 0)" + for _ in range(6): + geom = f"GEOMETRYCOLLECTION({geom})" + obj = GeometryCollectionModel(geom=geom) + msg = "WKT contains too many possible GeometryCollections." + # Spatial fields do not re-raise ValueError as ValidationError. + with self.assertRaisesMessage(ValueError, msg): + obj.full_clean() + + class GeoLookupTest(TestCase): fixtures = ["initial"] @@ -689,6 +737,15 @@ class GeoLookupTest(TestCase): # Just get SQL to avoid gating on connection.supports_raster. City.objects.filter(point__contained=geojson).query + @skipUnlessGISLookup("exact") + def test_lookup_against_nested_geometry_collection(self): + geom = "POINT(0 0)" + for _ in range(6): + geom = f"GEOMETRYCOLLECTION({geom})" + msg = "WKT contains too many possible GeometryCollections." + with self.assertRaisesMessage(ValueError, msg): + GeometryCollectionModel.objects.filter(geom=geom) + class GeoQuerySetTest(TestCase): # TODO: GeoQuerySet is removed, organize these test better. diff --git a/tests/gis_tests/geos_tests/test_geos_limit.py b/tests/gis_tests/geos_tests/test_geos_limit.py new file mode 100644 index 0000000000..1904436eaf --- /dev/null +++ b/tests/gis_tests/geos_tests/test_geos_limit.py @@ -0,0 +1,176 @@ +import struct + +from django.contrib.gis.geos import GEOSGeometry, WKTReader +from django.contrib.gis.geos.error import GEOSException +from django.contrib.gis.geos.prototypes.io import MAX_GEOM_COLLECTIONS +from django.test import SimpleTestCase + + +class GEOSLimitTest(SimpleTestCase): + def _generate_geometry_collection_payloads(self, depth): + def point(endian="<", type_code=1, dims=2, srid=None): + marker = b"\x01" if endian == "<" else b"\x00" + head = marker + struct.pack(f"{endian}I", type_code) + if srid is not None: + head += struct.pack(f"{endian}I", srid) + return head + struct.pack(f"{endian}{'d' * dims}", *((0.0,) * dims)) + + def layer(endian="<", type_code=7, srid=None): + marker = b"\x01" if endian == "<" else b"\x00" + head = marker + struct.pack(f"{endian}I", type_code) + if srid is not None: + head += struct.pack(f"{endian}I", srid) + return head + struct.pack(f"{endian}I", 1) + + def wkb(coll=7, child=1, dims=2, srid=None): + return layer(type_code=coll, srid=srid) * depth + point( + type_code=child, dims=dims, srid=srid + ) + + # (label, collection type code, child type code, child dims, srid, ... + # check_geos). + variants = [ + ("ISO WKB Z", 1007, 1001, 3, None, True), + ("ISO WKB M", 2007, 2001, 3, None, True), + ("ISO WKB ZM", 3007, 3001, 4, None, True), + ("EWKB Z", 0x80000007, 0x80000001, 3, None, True), + ("EWKB M", 0x40000007, 0x40000001, 3, None, True), + ("EWKB ZM", 0xC0000007, 0xC0000001, 4, None, True), + ("EWKB SRID", 0x20000007, 0x20000001, 2, 4326, True), + ("EWKB Z and SRID", 0xA0000007, 0xA0000001, 3, 4326, True), + ("EWKB ZM and SRID", 0xE0000007, 0xE0000001, 4, 4326, True), + # Undoc'd high-bit combinations accepted by some GEOS versions. + # These only verify that Django's limiter recognizes the normalized + # GeometryCollection type before GEOS parses the payload. + ("EWKB BBOX", 0x10000007, 0x10000001, 2, None, False), + ("EWKB BBOX and Z", 0x90000007, 0x90000001, 3, None, False), + ("EWKB BBOX and M", 0x50000007, 0x50000001, 3, None, False), + ("EWKB BBOX and ZM", 0xD0000007, 0xD0000001, 4, None, False), + ("EWKB BBOX and SRID", 0x30000007, 0x30000001, 2, 4326, False), + ("EWKB BBOX, Z, and SRID", 0xB0000007, 0xB0000001, 3, 4326, False), + ("EWKB BBOX, M, and SRID", 0x70000007, 0x70000001, 3, 4326, False), + ("EWKB BBOX, ZM, and SRID", 0xF0000007, 0xF0000001, 4, 4326, False), + ("EWKB unknown high flag", 0x01000007, 0x01000001, 2, None, False), + ] + binary = [ + (layer() * depth + point(), "little-endian WKB", True), + (layer(endian=">") * depth + point(endian=">"), "big-endian WKB", True), + ( + b"".join(layer(endian="<" if i % 2 == 0 else ">") for i in range(depth)) + + point(endian=">"), + "mixed-endian WKB", + True, + ), + ] + binary += [(wkb(c, ch, d, s), label, cg) for label, c, ch, d, s, cg in variants] + + payloads = [] + for data, label, check_geos in binary: + payloads += [ + (data.hex().upper(), f"{label}, uppercase hex string", check_geos), + (data.hex().encode("ascii"), f"{label}, lower hex bytes", check_geos), + (memoryview(data), f"{label}, memoryview", check_geos), + ] + wkt = "GEOMETRYCOLLECTION(" * depth + "POINT(0 0)" + ")" * depth + payloads += [(wkt, "WKT", True), (wkt.encode("ascii"), "WKT bytes", True)] + return payloads + + def test_geometry_collection_limit_exceeded(self): + msg = "contains too many possible GeometryCollections." + payloads = self._generate_geometry_collection_payloads(depth=6) + for payload, label, check_geos in payloads: + with self.subTest(payload=label): + with self.assertRaisesMessage(ValueError, msg): + GEOSGeometry(payload, max_geom_collections=5) + # Valid cases. + if check_geos: + GEOSGeometry(payload, max_geom_collections=6) + GEOSGeometry(payload, max_geom_collections=None) + + def test_wkt_geometry_collection_flat(self): + def wkt_payload_no_nesting(num_points): + # Many parentheses, but only one collection level. + return ( + "GEOMETRYCOLLECTION(" + + ",".join("POINT(0 0)" for _ in range(num_points)) + + ")" + ) + + GEOSGeometry(wkt_payload_no_nesting(num_points=5), max_geom_collections=1) + + def test_wkt_mixed_case_and_inner_whitespace_is_limited(self): + two_collections = ( + "GEOMETRYCOLLECTION ( " + "geometrycollection ( " + "POINT (0 0), POINT(1 1)" + ") )" + ) + msg = "WKT contains too many possible GeometryCollections." + with self.assertRaisesMessage(ValueError, msg): + GEOSGeometry(two_collections, max_geom_collections=1) + GEOSGeometry(two_collections, max_geom_collections=2) + + def test_from_ewkt_leading_whitespace_is_limited(self): + # from_ewkt() hands the part after the SRID to the low-level reader, + # so leading whitespace never passes through wkt_regex. + wkt = " " + "GEOMETRYCOLLECTION(" * 200 + "POINT(0 0)" + ")" * 200 + msg = "WKT contains too many possible GeometryCollections." + for value in wkt, wkt.encode(): + with self.subTest(value=value): + with self.assertRaisesMessage(ValueError, msg): + GEOSGeometry.from_ewkt(value) + + def test_wkt_dimension_marker_whitespace_is_limited(self): + def two_collections(separator): + collection = f"GEOMETRYCOLLECTION{separator}ZM" + return f"{collection}({collection}(POINT ZM (0 0 0 0)))" + + msg = "WKT contains too many possible GeometryCollections." + # GEOS accepts any amount of whitespace before the dimension marker. + for separator in "", " ", " ": + with self.subTest(separator=separator): + value = two_collections(separator) + with self.assertRaisesMessage(ValueError, msg): + GEOSGeometry(value, max_geom_collections=1) + GEOSGeometry(value, max_geom_collections=2) + + def test_wkt_reader_whitespace_is_limited(self): + # WKTReader.read() takes str and bytes directly, so the whitespace + # GEOS tolerates but wkt_regex rejects reaches the limiter. + reader = WKTReader() + msg = "WKT contains too many possible GeometryCollections." + for prefix in "", " ", "\t\n ": + for separator in "", " ", " ", "\t", "\n", " \t\n ": + collection = f"GEOMETRYCOLLECTION{separator}ZM" + point = "POINT ZM (0 0 0 0)" + depth = MAX_GEOM_COLLECTIONS + 1 + over = prefix + f"{collection}(" * depth + point + ")" * depth + with self.subTest(prefix=prefix, separator=separator): + for value in over, over.encode(): + with self.assertRaisesMessage(ValueError, msg): + reader.read(value) + under = f"{prefix}{collection}({point})" + self.assertEqual(reader.read(under).geom_type, "GeometryCollection") + + def test_non_collection_wkt_root_fast_path(self): + def make_geom(depth): + return "GEOMETRYCOLLECTION(" * depth + "POINT(0 0)" + ")" * depth + + invalid_wkt = "POLYGON(" + make_geom(6) + ")" + # Instead of raising a ValueError, a fast path skips the limit and + # depends on GEOS to reject collections found anywhere but the root. + with self.assertRaises(GEOSException): + GEOSGeometry(invalid_wkt, max_geom_collections=5) + + def test_malformed_multi_wkb_child_is_limited(self): + def make_invalid_geom(depth): + point = b"\x01" + struct.pack(" default applies + + fld = forms.GeometryField(max_geom_collections=5, widget=IgnoringWidget) + + def make_geom(depth): + return "GEOMETRYCOLLECTION(" * depth + "POINT(0 0)" + ")" * depth + + # The field's low limit (5) is ignored by the widget... + self.assertIsNotNone(fld.clean(make_geom(6))) + # ...but the default (198) still guards against deeper input. + with self.assertRaises(ValueError): + fld.clean(make_geom(MAX_GEOM_COLLECTIONS + 1)) + def test_null(self): "Testing GeometryField's handling of null (None) geometries." # Form fields, by default, are required (`required=True`) -- 2.44.4