Code

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#11245 closed (fixed)

contrib.gis.utils.GeoIP._check_query does not check for NULL pointer to _city or _country GeoIP libraries

Reported by: andrewfox Owned by: jbronn
Component: GIS Version: 1.0
Severity: Keywords: geodjango, gis, geoip, ctype
Cc: andrewfox Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description (last modified by jbronn)

In the _check_query method of geoip.py lines 344, 346, and 348 should not check self._city and/or self._country for 'is None' but instead for 'not self._city' or 'not self._country'. self._city and self._country are ctype wrapped pointers; so even if they returned NULL from the C library (e.g. fopen failure), self._city and self._country would still be not None but instead their pointer values would be None. Ctype checking for null pointers could be instead achieved by checking for 'not self._city'.

Referenced code (django/contrib/gis/utils/geoip.py (~line 344):

        # Extra checks for the existence of country and city databases.
        if city_or_country and self._country is None and self._city is None:
            raise GeoIPException('Invalid GeoIP country and city data files.')
        elif country and self._country is None:
            raise GeoIPException('Invalid GeoIP country data file: %s' % self._country_file)
        elif city and self._city is None:
            raise GeoIPException('Invalid GeoIP city data file: %s' % self._city_file)

Attachments (0)

Change History (5)

comment:1 Changed 5 years ago by andrewfox

  • Cc andrewfox added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 5 years ago by andrewfox

Lines actually 222, 224, and 226.

comment:3 Changed 5 years ago by jbronn

  • Component changed from Contrib apps to GIS
  • Description modified (diff)
  • Owner changed from nobody to jbronn

comment:4 Changed 5 years ago by jbronn

  • Resolution set to fixed
  • Status changed from new to closed

(In [10979]) Fixed #11245, #11246 -- Fixed validity check of GeoIP pointers and leaking of their references; also clarified initialization, fixed a stale test, added comments about version compatibility, and did some whitespace cleanup.

comment:5 Changed 5 years ago by jbronn

(In [10980]) [1.0.X] Fixed #11245, #11246 -- Fixed validity check of GeoIP pointers and leaking of their references; also clarified initialization, fixed a stale test, added comments about version compatibility, and did some whitespace cleanup.

Backport of r10979 from trunk.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.