geoip: Fix memory leak when using GeoIPRecord

GeoIP_record_by_ipnum{,_v6} returns allocated pointer to
GeoIPRecord with allocated city, region & postal_code which is
not freed after the call.

Fixed by xstrdup-ing required GeoIPRecord member (city/region) and
after calling GeoIPRecord_delete to free the geoip record.

Of course it is needed to also free obtained city/region in netsniff-ng,
astraceroute & flowtop tools.

Fixes #169
Signed-off-by: Vadim Kochan <vadim4j@gmail.com>
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>

6 files changed, 141 insertions, 33 deletions

diff --git a/astraceroute.c b/astraceroute.c
index 98b97e4..ca47ba0 100644
--- a/astraceroute.c
+++ b/astraceroute.c
@@ -457,7 +457,8 @@ static void handle_ipv4(uint8_t *packet, size_t len __maybe_unused,
 	struct iphdr *iph = (struct iphdr *) packet;
 	struct sockaddr_in sd;
 	struct hostent *hent;
-	const char *as, *country, *city;
+	const char *as, *country;
+	char *city;
 
 	memset(hbuff, 0, sizeof(hbuff));
 	memset(&sd, 0, sizeof(sd));
@@ -489,6 +490,8 @@ static void handle_ipv4(uint8_t *packet, size_t len __maybe_unused,
 	}
 	if (latitude)
 		printf(" (%f/%f)", geoip4_latitude(&sd), geoip4_longitude(&sd));
+
+	free(city);
 }
 
 static int check_ipv6(uint8_t *packet, size_t len, int ttl __maybe_unused,
@@ -524,7 +527,8 @@ static void handle_ipv6(uint8_t *packet, size_t len __maybe_unused,
 	struct ip6_hdr *ip6h = (struct ip6_hdr *) packet;
 	struct sockaddr_in6 sd;
 	struct hostent *hent;
-	const char *as, *country, *city;
+	const char *as, *country;
+	char *city;
 
 	memset(hbuff, 0, sizeof(hbuff));
 	memset(&sd, 0, sizeof(sd));
@@ -556,6 +560,8 @@ static void handle_ipv6(uint8_t *packet, size_t len __maybe_unused,
 	}
 	if (latitude)
 		printf(" (%f/%f)", geoip6_latitude(&sd), geoip6_longitude(&sd));
+
+	free(city);
 }
 
 static void show_trace_info(struct ctx *ctx, const struct sockaddr_storage *ss,
diff --git a/flowtop.c b/flowtop.c
index 2491de7..1caae9e 100644
--- a/flowtop.c
+++ b/flowtop.c
@@ -682,7 +682,7 @@ flow_entry_geo_city_lookup_generic(struct flow_entry *n,
 {
 	struct sockaddr_in sa4;
 	struct sockaddr_in6 sa6;
-	const char *city = NULL;
+	char *city = NULL;
 
 	switch (n->l3_proto) {
 	default:
@@ -706,6 +706,8 @@ flow_entry_geo_city_lookup_generic(struct flow_entry *n,
 		        sizeof(n->city_src));
 	else
 		SELFLD(dir, city_src, city_dst)[0] = '\0';
+
+	free(city);
 }
 
 static void
diff --git a/geoip.c b/geoip.c
index 917b1a7..d95305c 100644
--- a/geoip.c
+++ b/geoip.c
@@ -73,8 +73,6 @@ static GeoIP *gi4_asname = NULL, *gi6_asname = NULL;
 static GeoIP *gi4_country = NULL, *gi6_country = NULL;
 static GeoIP *gi4_city = NULL, *gi6_city = NULL;
 
-static GeoIPRecord empty = { NULL };
-
 static char *servers[16] = { NULL };
 
 #define CITYV4		(1 << 0)
@@ -261,14 +259,14 @@ static GeoIPRecord *geoip4_get_record(struct sockaddr_in *sa)
 {
 	bug_on(gi4_city == NULL);
 
-	return GeoIP_record_by_ipnum(gi4_city, ntohl(sa->sin_addr.s_addr)) ? : &empty;
+	return GeoIP_record_by_ipnum(gi4_city, ntohl(sa->sin_addr.s_addr));
 }
 
 static GeoIPRecord *geoip6_get_record(struct sockaddr_in6 *sa)
 {
 	bug_on(gi6_city == NULL);
 
-	return GeoIP_record_by_ipnum_v6(gi6_city, sa->sin6_addr) ? : &empty;
+	return GeoIP_record_by_ipnum_v6(gi6_city, sa->sin6_addr);
 }
 
 const char *geoip4_as_name(struct sockaddr_in *sa)
@@ -287,42 +285,126 @@ const char *geoip6_as_name(struct sockaddr_in6 *sa)
 
 float geoip4_longitude(struct sockaddr_in *sa)
 {
-	return geoip4_get_record(sa)->longitude;
+	GeoIPRecord *record;
+	float longitude;
+
+	record = geoip4_get_record(sa);
+	if (!record)
+		return 0;
+
+	longitude = record->longitude;
+
+	GeoIPRecord_delete(record);
+	return longitude;
 }
 
 float geoip4_latitude(struct sockaddr_in *sa)
 {
-	return geoip4_get_record(sa)->latitude;
+	GeoIPRecord *record;
+	float latitude;
+
+	record = geoip4_get_record(sa);
+	if (!record)
+		return 0;
+
+	latitude = record->latitude;
+
+	GeoIPRecord_delete(record);
+	return latitude;
 }
 
 float geoip6_longitude(struct sockaddr_in6 *sa)
 {
-	return geoip6_get_record(sa)->longitude;
+	GeoIPRecord *record;
+	float longitude;
+
+	record = geoip6_get_record(sa);
+	if (!record)
+		return 0;
+
+	longitude = record->longitude;
+
+	GeoIPRecord_delete(record);
+	return longitude;
 }
 
 float geoip6_latitude(struct sockaddr_in6 *sa)
 {
-	return geoip6_get_record(sa)->latitude;
+	GeoIPRecord *record;
+	float latitude;
+
+	record = geoip6_get_record(sa);
+	if (!record)
+		return 0;
+
+	latitude = record->latitude;
+
+	GeoIPRecord_delete(record);
+	return latitude;
 }
 
-const char *geoip4_city_name(struct sockaddr_in *sa)
+char *geoip4_city_name(struct sockaddr_in *sa)
 {
-	return geoip4_get_record(sa)->city;
+	GeoIPRecord *record;
+	char *city = NULL;
+
+	record = geoip4_get_record(sa);
+	if (!record)
+		return NULL;
+
+	if (record->city)
+		city = xstrdup(record->city);
+
+	GeoIPRecord_delete(record);
+	return city;
 }
 
-const char *geoip6_city_name(struct sockaddr_in6 *sa)
+char *geoip6_city_name(struct sockaddr_in6 *sa)
 {
-	return geoip6_get_record(sa)->city;
+	GeoIPRecord *record;
+	char *city = NULL;
+
+	record = geoip6_get_record(sa);
+	if (!record)
+		return NULL;
+
+	if (record->city)
+		city = xstrdup(record->city);
+
+	GeoIPRecord_delete(record);
+	return city;
 }
 
-const char *geoip4_region_name(struct sockaddr_in *sa)
+char *geoip4_region_name(struct sockaddr_in *sa)
 {
-	return geoip4_get_record(sa)->region;
+	GeoIPRecord *record;
+	char *region = NULL;
+
+	record = geoip4_get_record(sa);
+	if (!record)
+		return NULL;
+
+	if (record->region)
+		region = xstrdup(record->region);
+
+	GeoIPRecord_delete(record);
+	return region;
 }
 
-const char *geoip6_region_name(struct sockaddr_in6 *sa)
+char *geoip6_region_name(struct sockaddr_in6 *sa)
 {
-	return geoip6_get_record(sa)->region;
+	GeoIPRecord *record;
+	char *region = NULL;
+
+	record = geoip6_get_record(sa);
+	if (!record)
+		return NULL;
+
+	if (record->region)
+		region = xstrdup(record->region);
+
+	GeoIPRecord_delete(record);
+	return region;
 }
 
 const char *geoip4_country_name(struct sockaddr_in *sa)
diff --git a/geoip.h b/geoip.h
index 5d5c221..cdeef19 100644
--- a/geoip.h
+++ b/geoip.h
@@ -11,10 +11,10 @@
 extern void init_geoip(int enforce);
 extern void update_geoip(void);
 extern int geoip_working(void);
-extern const char *geoip4_city_name(struct sockaddr_in *sa);
-extern const char *geoip6_city_name(struct sockaddr_in6 *sa);
-extern const char *geoip4_region_name(struct sockaddr_in *sa);
-extern const char *geoip6_region_name(struct sockaddr_in6 *sa);
+extern char *geoip4_city_name(struct sockaddr_in *sa);
+extern char *geoip6_city_name(struct sockaddr_in6 *sa);
+extern char *geoip4_region_name(struct sockaddr_in *sa);
+extern char *geoip6_region_name(struct sockaddr_in6 *sa);
 extern const char *geoip4_country_name(struct sockaddr_in *sa);
 extern const char *geoip6_country_name(struct sockaddr_in6 *sa);
 extern const char *geoip4_country_code3_name(struct sockaddr_in *sa);
diff --git a/proto_ipv4.c b/proto_ipv4.c
index edce915..e2d22d8 100644
--- a/proto_ipv4.c
+++ b/proto_ipv4.c
@@ -40,7 +40,8 @@ static void ipv4(struct pkt_buff *pkt)
 	uint8_t *opt, *trailer;
 	unsigned int trailer_len = 0;
 	ssize_t opts_len, opt_len;
-	const char *city, *region, *country;
+	const char *country;
+	char *city, *region;
 
 	if (!ip)
 		return;
@@ -101,20 +102,28 @@ static void ipv4(struct pkt_buff *pkt)
 		tprintf("\t[ Geo (");
 		if ((country = geoip4_country_name(&sas))) {
 			tprintf("%s", country);
-			if ((region = geoip4_region_name(&sas)))
+			if ((region = geoip4_region_name(&sas))) {
 				tprintf(" / %s", region);
-			if ((city = geoip4_city_name(&sas)))
+				xfree(region);
+			}
+			if ((city = geoip4_city_name(&sas))) {
 				tprintf(" / %s", city);
+				xfree(city);
+			}
 		} else {
 			tprintf("local");
 		}
 		tprintf(" => ");
 		if ((country = geoip4_country_name(&sad))) {
 			tprintf("%s", country);
-			if ((region = geoip4_region_name(&sad)))
+			if ((region = geoip4_region_name(&sad))) {
 				tprintf(" / %s", region);
-			if ((city = geoip4_city_name(&sad)))
+				xfree(region);
+			}
+			if ((city = geoip4_city_name(&sad))) {
 				tprintf(" / %s", city);
+				xfree(city);
+			}
 		} else {
 			tprintf("local");
 		}
diff --git a/proto_ipv6.c b/proto_ipv6.c
index ba3f6c8..e3e8f68 100644
--- a/proto_ipv6.c
+++ b/proto_ipv6.c
@@ -27,7 +27,8 @@ void ipv6(struct pkt_buff *pkt)
 	char dst_ip[INET6_ADDRSTRLEN];
 	struct ipv6hdr *ip = (struct ipv6hdr *) pkt_pull(pkt, sizeof(*ip));
 	struct sockaddr_in6 sas, sad;
-	const char *city, *region, *country;
+	char *city, *region;
+	const char *country;
 
 	if (ip == NULL)
 		return;
@@ -62,20 +63,28 @@ void ipv6(struct pkt_buff *pkt)
 		tprintf("\t[ Geo (");
 		if ((country = geoip6_country_name(&sas))) {
 			tprintf("%s", country);
-			if ((region = geoip6_region_name(&sas)))
+			if ((region = geoip6_region_name(&sas))) {
 				tprintf(" / %s", region);
-			if ((city = geoip6_city_name(&sas)))
+				xfree(region);
+			}
+			if ((city = geoip6_city_name(&sas))) {
 				tprintf(" / %s", city);
+				xfree(city);
+			}
 		} else {
 			tprintf("local");
 		}
 		tprintf(" => ");
 		if ((country = geoip6_country_name(&sad))) {
 			tprintf("%s", country);
-			if ((region = geoip6_region_name(&sad)))
+			if ((region = geoip6_region_name(&sad))) {
 				tprintf(" / %s", region);
-			if ((city = geoip6_city_name(&sad)))
+				xfree(region);
+			}
+			if ((city = geoip6_city_name(&sad))) {
 				tprintf(" / %s", city);
+				xfree(city);
+			}
 		} else {
 			tprintf("local");
 		}