From 8fd19eefa46b313673d6ba2a0194a68674ef5ea9 Mon Sep 17 00:00:00 2001 From: Vadim Kochan Date: Sat, 4 Feb 2017 11:56:14 +0200 Subject: 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 Signed-off-by: Tobias Klauser --- astraceroute.c | 10 ++++- flowtop.c | 4 +- geoip.c | 114 +++++++++++++++++++++++++++++++++++++++++++++++++-------- geoip.h | 8 ++-- proto_ipv4.c | 19 +++++++--- proto_ipv6.c | 19 +++++++--- 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)) ? : ∅ + 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) ? : ∅ + 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"); } -- cgit v1.2.3-54-g00ecf