php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login

go to bug id or search bugs for

Bug #68277 passing NULL in place of GeoIP struct lead to segfault in libGeoIP
Submitted: 2014-10-21 14:48 UTC Modified: 2021-08-26 14:28 UTC
Votes:4
Avg. Score:4.5 ± 0.5
Reproduced:4 of 4 (100.0%)
Same Version:1 (25.0%)
Same OS:3 (75.0%)
From: slim at inbox dot lv Assigned:
Status: Verified Package: geoip (PECL)
PHP Version: 5.4.34 OS: linux
Private report: No CVE-ID: None
View Developer Edit
 [2014-10-21 14:48 UTC] slim at inbox dot lv 
Description:
------------
in function geoip_record_by_name() after gi = GeoIP_open_type() `gi` must be checked for NULL before gir = GeoIP_record_by_name(gi, hostname);

libgeoip does not check gi and segfaults



Test script:
---------------
any broken /usr/share/GeoIP/GeoIPCity.dat file leads to segfault in libGeoIP

Expected result:
----------------
do not crash

Actual result:
--------------
segfault

Patches

patch-geoip-open-type (last revision 2014-10-23 15:22 UTC by phpbugs at joern dot heissler dot de)
geoip-1.1.0-city.patch (last revision 2014-10-23 13:08 UTC by slim at inbox dot lv)
patch-geoip_record_by_name (last revision 2014-10-23 02:47 UTC by phpbugs at joern dot heissler dot de)

Pull Requests

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2014-10-23 02:50 UTC] phpbugs at joern dot heissler dot de 
Hi,
I submitted a patch. Please review it carefully for it's almost 5am *yawn*
 [2014-10-23 03:03 UTC] phpbugs at joern dot heissler dot de 
Oh, and also see the GeoIP_db_avail function please. It checks only if a file exists, but these two database revisions share the same filename:

GeoIPDBFileName[GEOIP_CITY_EDITION_REV0] = _GeoIP_full_path_to(
    "GeoIPCity.dat");
GeoIPDBFileName[GEOIP_CITY_EDITION_REV1] = _GeoIP_full_path_to(
    "GeoIPCity.dat");

On my setup REV1 is said to be available, but apparently I'm using REV0 files. Trying to open my valid REV0 file as REV1 failed and caused a segfault too.
 [2014-10-23 13:07 UTC] slim at inbox dot lv 
the patch is wrong as you see
Check my one.
BTW gi need to be checked for NULL before every GeoIP_*_by_name() function !
 [2014-10-23 15:20 UTC] phpbugs at joern dot heissler dot de 
I don't see how my patch is wrong. Please explain.
It's certainly incomplete though, as it fixes only one occurrence of the bug. At a closer look, I think this bug has been repeated about a dozen times. I refactored the code a bit in a new patch.

Your patch is wrong and only works by accident:
If GeoIP_db_avail(GEOIP_CITY_EDITION_REV1) is true, you'll next GeoIP_open_type(GEOIP_CITY_EDITION_REV0, GEOIP_STANDARD).

These are different editions, you shouldn't check availability of one and open the other.
 [2014-10-27 13:05 UTC] slim at inbox dot lv 
can You please push your patch to pecl-geoip and release next version ?
 [2014-11-20 20:53 UTC] anthon at piwik dot org 
Previously reported in https://bugs.php.net/bug.php?id=67231. Please mark 67231 as a dupe since there appears to be more active discussion here. I'll comment on the proposed patches once I have a chance to review.
 [2015-04-17 02:50 UTC] goschwald at maxmind dot com 
It would be great to see this issue fixed. It was reported upstream here:

https://github.com/maxmind/geoip-api-c/issues/59

GeoIP_open_type will return NULL if the open fails, which can happen even if GeoIP_db_avail returns true. The latter only checks that the file exists and does not check if it is the right type.
 [2015-04-22 11:13 UTC] ab@php.net 
Related To: Bug #69496
 [2021-08-26 14:28 UTC] cmb@php.net
-Status: Open +Status: Verified
 [2021-08-26 14:28 UTC] cmb@php.net 
This appears to have not been fixed yet.  I suggest to submit a
respective pull request[1] for (hopefully) better visibility.

[1] <https://github.com/php/pecl-networking-geoip/pulls>
 [2021-08-26 14:31 UTC] cmb@php.net 
Related To: Bug #69216
 [2021-08-26 14:32 UTC] cmb@php.net 
Related To: Bug #70337
 [2021-09-17 10:50 UTC] slim at inbox dot lv 
EOL of geoip is scheduled to May 2022
https://blog.maxmind.com/2020/06/01/retirement-of-geoip-legacy-downloadable-databases-in-may-2022/

no much sense to fix this now after 7 years of coma
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved. 		Last updated: Mon Nov 25 01:01:31 2024 UTC