ipc: freebsd: avoid leaking memory in kernel_get_device()

Primarily, front-load validation of an allowed-ip entry to before we
allocate `aip`, so that we don't need to free() it if we end up skipping
this entry.  Assert that `aip` is NULL after we exit the loop, as we
should have transfered ownership to the `peer` or freed it in all paths
through the allowed-ip loop.

FreeBSD-Coverity:	1500405
Signed-off-by: Kyle Evans <kevans@FreeBSD.org>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>

diff --git a/src/ipc-freebsd.h b/src/ipc-freebsd.h
index b5be15b82140140e852c70f18d3a04c6841a40e4..b78b6c8960cd6aac8cb951790edb0a455b3f0187 100644 (file)
--- a/src/ipc-freebsd.h
+++ b/src/ipc-freebsd.h
@@ -4,6 +4,7 @@
  *
  */
 
+#include <assert.h>
 #include <sys/nv.h>
 #include <sys/sockio.h>
 #include <dev/wg/if_wg.h>
@@ -118,7 +119,7 @@ static int kernel_get_device(struct wgdevice **device, const char *ifname)
                goto skip_peers;
        for (i = 0; i < peer_count; ++i) {
                struct wgpeer *peer;
-               struct wgallowedip *aip;
+               struct wgallowedip *aip = NULL;
                const nvlist_t *const *nvl_aips;
                size_t aip_count, j;
 
@@ -169,11 +170,13 @@ static int kernel_get_device(struct wgdevice **device, const char *ifname)
                if (!aip_count || !nvl_aips)
                        goto skip_allowed_ips;
                for (j = 0; j < aip_count; ++j) {
+                       if (!nvlist_exists_number(nvl_aips[j], "cidr"))
+                               continue;
+                       if (!nvlist_exists_binary(nvl_aips[j], "ipv4") && !nvlist_exists_binary(nvl_aips[j], "ipv6"))
+                               continue;
                        aip = calloc(1, sizeof(*aip));
                        if (!aip)
                                goto err_allowed_ips;
-                       if (!nvlist_exists_number(nvl_aips[j], "cidr"))
-                               continue;
                        number = nvlist_get_number(nvl_aips[j], "cidr");
                        if (nvlist_exists_binary(nvl_aips[j], "ipv4")) {
                                binary = nvlist_get_binary(nvl_aips[j], "ipv4", &size);
@@ -184,7 +187,8 @@ static int kernel_get_device(struct wgdevice **device, const char *ifname)
                                aip->family = AF_INET;
                                aip->cidr = number;
                                memcpy(&aip->ip4, binary, sizeof(aip->ip4));
-                       } else if (nvlist_exists_binary(nvl_aips[j], "ipv6")) {
+                       } else {
+                               assert(nvlist_exists_binary(nvl_aips[j], "ipv6"));
                                binary = nvlist_get_binary(nvl_aips[j], "ipv6", &size);
                                if (!binary || number > 128) {
                                        ret = EINVAL;
@@ -193,14 +197,14 @@ static int kernel_get_device(struct wgdevice **device, const char *ifname)
                                aip->family = AF_INET6;
                                aip->cidr = number;
                                memcpy(&aip->ip6, binary, sizeof(aip->ip6));
-                       } else
-                               continue;
+                       }
 
                        if (!peer->first_allowedip)
                                peer->first_allowedip = aip;
                        else
                                peer->last_allowedip->next_allowedip = aip;
                        peer->last_allowedip = aip;
+                       aip = NULL;
                        continue;
 
                err_allowed_ips:
@@ -209,6 +213,9 @@ static int kernel_get_device(struct wgdevice **device, const char *ifname)
                        free(aip);
                        goto err_peer;
                }
+
+               /* Nothing leaked, hopefully -- ownership transferred or aip freed. */
+               assert(aip == NULL);
        skip_allowed_ips:
                if (!dev->first_peer)
                        dev->first_peer = peer;