#9 geoip-c: geoiplookup can crash

[Klaus Heinz]

Hi,

geoiplookup can crash with a segmentation fault if
the data file either does not exist or does not have the
correct format.

This can be easily tested by supplying a non-existing
file or by supplying an empty file for option "-f".

The appended patches address this issue.

ciao
Klaus

[Klaus Heinz]

crash fix for geoiplookup.c

[Klaus Heinz]

crash fix for GeoIP.c

[Klaus Heinz]

Logged In: YES
user_id=881982
Originator: YES

File Added: patch-ad

[BZ]

Logged In: YES
user_id=245475
Originator: NO

Hi Klaus,

much thanks for the patches.
Similar fixes are already in cvs.

[Klaus Heinz]

Hi,

sorry it took a while to respond.

Looking at GeoIP 1.4.5 I see that the first part of my patch-ac for
apps/geoiplookup.c is still missing.

Do you _really_ want to store the variable "custom_directory" for later if
"custom_file" is non-NULL?
To me, this would only make sense if the comparison involved "custom_directory"
instead of "custom_file".

line 67 of apps/geoiplookup.c:

if (custom_file != NULL) {
GeoIP_setup_custom_directory(custom_directory);
}

What am I missing?

ciao
Klaus

[Klaus Heinz]

Hi,

sorry it took a while to respond.

Looking at GeoIP 1.4.5 I see that the first part of my patch-ac for
apps/geoiplookup.c is still missing.

Do you _really_ want to store the variable "custom_directory" for later if
"custom_file" is non-NULL?
To me, this would only make sense if the comparison involved "custom_directory"
instead of "custom_file".

line 67 of apps/geoiplookup.c:

if (custom_file != NULL) {
GeoIP_setup_custom_directory(custom_directory);
}

What am I missing?

ciao
Klaus

[BZ]

Hi Klaus,

thanks for heads up. I have replaced custom_file with custom_directory in the cvs tree.

The second part from your patch is not really correct. It just hide the error for some more cases. Seems that you use only the country database, but there are region, org, city, ... and domain databases too. And there is a upper limit which has the same problem. Right now my only idea to fix this correct, is a database integrity check before the start. But that is way to expensive for most users.

Maybe we can find a fast test that works most of the time.

Thanks, have a nice day.