diff options
| author |   Kyle Evans <kevans@FreeBSD.org> | 2022-11-03 12:59:01 -0500 |
|---|---|---|
| committer |   Jason A. Donenfeld <Jason@zx2c4.com> | 2022-11-03 19:57:21 +0100 |
| commit | dbf49a7d17d69b8af365ce2521df110a4710c819 (patch) | |
| tree | aa779b1d9233c4f5838e57f0e542a70b6d1f13a8 /src/ipc-freebsd.h | |
| parent | show: apply const to right part of pointer (diff) | |
| download | wireguard-tools-dbf49a7d17d69b8af365ce2521df110a4710c819.tar.xz wireguard-tools-dbf49a7d17d69b8af365ce2521df110a4710c819.zip | |
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>
Diffstat (limited to '')
| -rw-r--r-- | src/ipc-freebsd.h | 19 |
1 files changed, 13 insertions, 6 deletions
diff --git a/src/ipc-freebsd.h b/src/ipc-freebsd.h index b5be15b..b78b6c8 100644 --- 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; |