From: Steffan Karger Date: Mon, 23 Nov 2015 20:58:55 +0000 (+0100) Subject: Fix memory leak in add_option() by simplifying get_ipv6_addr X-Git-Tag: v2.4_alpha1~192 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=4f19cd1ddde3929d4a715ad59cd603eff16c66bf;p=thirdparty%2Fopenvpn.git Fix memory leak in add_option() by simplifying get_ipv6_addr If get_ipv6_addr() would fail *after* allocating memory for ipv6_local, add_option() would fail to free that memory. The fix here is to remove the allocation from get_ipv6_addr(), and create a separate function for the strip-and-allocate, such that failures are easier to handle. v2 - remove free(options->ifconfig_ipv6_local), since that is now handled by a garbage collector. Memory leak found by coverity (in 2011!). Signed-off-by: Steffan Karger Acked-by: Gert Doering Message-Id: <1448312335-25908-1-git-send-email-steffan@karger.me> URL: http://article.gmane.org/gmane.network.openvpn.devel/10573 Signed-off-by: Gert Doering (cherry picked from commit 7e618994f3112ff4b29b9f08d087fb558636a6af) --- diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 207e9daae..36290a015 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -949,13 +949,11 @@ get_ip_addr (const char *ip_string, int msglevel, bool *error) * "/nn" is optional, default to /64 if missing * * return true if parsing succeeded, modify *network and *netbits - * return address part without "/nn" in *printable_ipv6 (if != NULL) */ bool get_ipv6_addr( const char * prefix_str, struct in6_addr *network, - unsigned int * netbits, char ** printable_ipv6, int msglevel ) + unsigned int * netbits, int msglevel) { - int rc; char * sep, * endp; int bits; struct in6_addr t_network; @@ -982,21 +980,14 @@ get_ipv6_addr( const char * prefix_str, struct in6_addr *network, if ( sep != NULL ) *sep = '\0'; - rc = inet_pton( AF_INET6, prefix_str, &t_network ); - - if ( rc == 1 && printable_ipv6 != NULL ) - { - *printable_ipv6 = string_alloc( prefix_str, NULL ); - } - - if ( sep != NULL ) *sep = '/'; - - if ( rc != 1 ) + if ( inet_pton( AF_INET6, prefix_str, &t_network ) != 1 ) { msg (msglevel, "IPv6 prefix '%s': invalid IPv6 address", prefix_str); return false; } + if ( sep != NULL ) *sep = '/'; + if ( netbits != NULL ) { *netbits = bits; @@ -1008,12 +999,35 @@ get_ipv6_addr( const char * prefix_str, struct in6_addr *network, return true; /* parsing OK, values set */ } +/** + * Returns newly allocated string containing address part without "/nn". + * + * If gc != NULL, the allocated memory is registered in the supplied gc. + */ +static char * +get_ipv6_addr_no_netbits (const char *addr, struct gc_arena *gc) +{ + const char *end = strchr (addr, '/'); + char *ret = NULL; + if (NULL == end) + { + ret = string_alloc (addr, gc); + } + else + { + size_t len = end - addr; + ret = gc_malloc (len + 1, true, gc); + memcpy (ret, addr, len); + } + return ret; +} + static bool ipv6_addr_safe_hexplusbits( const char * ipv6_prefix_spec ) { struct in6_addr t_addr; unsigned int t_bits; - return get_ipv6_addr( ipv6_prefix_spec, &t_addr, &t_bits, NULL, M_WARN ); + return get_ipv6_addr( ipv6_prefix_spec, &t_addr, &t_bits, M_WARN ); } static char * @@ -1257,7 +1271,7 @@ option_iroute_ipv6 (struct options *o, ALLOC_OBJ_GC (ir, struct iroute_ipv6, &o->gc); - if ( !get_ipv6_addr (prefix_str, &ir->network, &ir->netbits, NULL, msglevel )) + if ( !get_ipv6_addr (prefix_str, &ir->network, &ir->netbits, msglevel )) { msg (msglevel, "in --iroute-ipv6 %s: Bad IPv6 prefix specification", prefix_str); @@ -4162,7 +4176,7 @@ add_option (struct options *options, struct in6_addr remote = IN6ADDR_ANY_INIT; VERIFY_PERMISSION (OPT_P_GENERAL); if (p[1]) - get_ipv6_addr (p[1], &remote, NULL, NULL, M_WARN); + get_ipv6_addr (p[1], &remote, NULL, M_WARN); get_default_gateway(&rgi); get_default_gateway_ipv6(&rgi6, &remote); print_default_gateway(M_INFO, &rgi, &rgi6); @@ -4417,10 +4431,9 @@ add_option (struct options *options, else if (streq (p[0], "ifconfig-ipv6") && p[1] && p[2] && !p[3]) { unsigned int netbits; - char * ipv6_local; VERIFY_PERMISSION (OPT_P_UP); - if ( get_ipv6_addr( p[1], NULL, &netbits, &ipv6_local, msglevel ) && + if ( get_ipv6_addr( p[1], NULL, &netbits, msglevel ) && ipv6_addr_safe( p[2] ) ) { if ( netbits < 64 || netbits > 124 ) @@ -4429,11 +4442,7 @@ add_option (struct options *options, goto err; } - if (options->ifconfig_ipv6_local) - /* explicitly ignoring this is a const char */ - free ((char *) options->ifconfig_ipv6_local); - - options->ifconfig_ipv6_local = ipv6_local; + options->ifconfig_ipv6_local = get_ipv6_addr_no_netbits (p[1], &options->gc); options->ifconfig_ipv6_netbits = netbits; options->ifconfig_ipv6_remote = p[2]; } @@ -5497,7 +5506,7 @@ add_option (struct options *options, unsigned int netbits = 0; VERIFY_PERMISSION (OPT_P_GENERAL); - if ( ! get_ipv6_addr (p[1], &network, &netbits, NULL, lev) ) + if ( ! get_ipv6_addr (p[1], &network, &netbits, lev) ) { msg (msglevel, "error parsing --server-ipv6 parameter"); goto err; @@ -5608,7 +5617,7 @@ add_option (struct options *options, unsigned int netbits = 0; VERIFY_PERMISSION (OPT_P_GENERAL); - if ( ! get_ipv6_addr (p[1], &network, &netbits, NULL, lev ) ) + if ( ! get_ipv6_addr (p[1], &network, &netbits, lev ) ) { msg (msglevel, "error parsing --ifconfig-ipv6-pool parameters"); goto err; @@ -5879,7 +5888,7 @@ add_option (struct options *options, VERIFY_PERMISSION (OPT_P_INSTANCE); - if ( ! get_ipv6_addr( p[1], &local, &netbits, NULL, msglevel ) ) + if ( ! get_ipv6_addr( p[1], &local, &netbits, msglevel ) ) { msg (msglevel, "cannot parse --ifconfig-ipv6-push addresses"); goto err; @@ -5887,7 +5896,7 @@ add_option (struct options *options, if ( p[2] ) { - if ( !get_ipv6_addr( p[2], &remote, NULL, NULL, msglevel ) ) + if ( !get_ipv6_addr( p[2], &remote, NULL, msglevel ) ) { msg( msglevel, "cannot parse --ifconfig-ipv6-push addresses"); goto err; @@ -5897,7 +5906,7 @@ add_option (struct options *options, { if ( ! options->ifconfig_ipv6_local || ! get_ipv6_addr( options->ifconfig_ipv6_local, &remote, - NULL, NULL, msglevel ) ) + NULL, msglevel ) ) { msg( msglevel, "second argument to --ifconfig-ipv6-push missing and no global --ifconfig-ipv6 address set"); goto err; diff --git a/src/openvpn/options.h b/src/openvpn/options.h index d1333d38a..ebc0591f5 100644 --- a/src/openvpn/options.h +++ b/src/openvpn/options.h @@ -775,8 +775,7 @@ void options_string_import (struct options *options, struct env_set *es); bool get_ipv6_addr( const char * prefix_str, struct in6_addr *network, - unsigned int * netbits, char ** printable_ipv6, - int msglevel ); + unsigned int * netbits, int msglevel ); #endif diff --git a/src/openvpn/route.c b/src/openvpn/route.c index 58b16c0f8..d06018730 100644 --- a/src/openvpn/route.c +++ b/src/openvpn/route.c @@ -380,7 +380,7 @@ init_route_ipv6 (struct route_ipv6 *r6, { CLEAR (*r6); - if ( !get_ipv6_addr( r6o->prefix, &r6->network, &r6->netbits, NULL, M_WARN )) + if ( !get_ipv6_addr( r6o->prefix, &r6->network, &r6->netbits, M_WARN )) goto fail; /* gateway */