bateast
/
wireguard-tools
Archived
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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>
This commit is contained in:
Kyle Evans 2022-11-03 12:59:01 -05:00 committed by Jason A. Donenfeld
parent ca2e89ff21
commit dbf49a7d17
1 changed files with 13 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
src/ipc-freebsd.h
View File

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