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
Welcome back! If you're the original bug submitter, here's where you can edit the bug or add additional notes.
If you forgot your password, you can retrieve your password here.
Password:
Status:
Package:
Bug Type:
Summary:
From: slim at inbox dot lv
New email:
PHP Version: OS:

 

 [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-2025 The PHP Group
All rights reserved. 		Last updated: Tue Feb 18 01:01:29 2025 UTC