From 4ebd1c524ac7a50e5dee6b63bfc89001dd37e450 Mon Sep 17 00:00:00 2001 From: Tobias Brunner Date: Thu, 9 Jul 2026 18:46:20 +0200 Subject: [PATCH] host: Do some proper validation when parsing CIDR-style subnets The previous code returned negative or too large values (e.g. /33 or /-1) to the caller, which some would then use unchecked. In particular the attribute parser in the vici plugin would use it directly to generate a subnet mask using shifts by `32 - mask`, which could trigger undefined behavior. Fixes: 65697c27345b ("Added a CIDR notation based host constructor") --- src/libstrongswan/networking/host.c | 25 +++++++++++++++++++--- src/libstrongswan/tests/suites/test_host.c | 16 ++++++++++---- 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/src/libstrongswan/networking/host.c b/src/libstrongswan/networking/host.c index fae90b05b..a63673a76 100644 --- a/src/libstrongswan/networking/host.c +++ b/src/libstrongswan/networking/host.c @@ -17,6 +17,8 @@ * for more details. */ +#include + #include "host.h" #include @@ -571,8 +573,9 @@ bool host_create_from_range(char *string, host_t **from, host_t **to) */ host_t *host_create_from_subnet(char *string, int *bits) { - char *pos, buf[64]; + char *pos, *end, buf[64]; host_t *net; + long netbits; pos = strchr(string, '/'); if (pos) @@ -583,8 +586,24 @@ host_t *host_create_from_subnet(char *string, int *bits) } strncpy(buf, string, pos - string); buf[pos - string] = '\0'; - *bits = atoi(pos + 1); - return host_create_from_string(buf, 0); + errno = 0; + netbits = strtol(pos + 1, &end, 10); + if (errno || end == pos + 1 || *end || netbits < 0 || netbits > 128) + { + return NULL; + } + net = host_create_from_string(buf, 0); + if (!net) + { + return NULL; + } + if (net->get_family(net) == AF_INET && netbits > 32) + { + net->destroy(net); + return NULL; + } + *bits = netbits; + return net; } net = host_create_from_string(string, 0); if (net) diff --git a/src/libstrongswan/tests/suites/test_host.c b/src/libstrongswan/tests/suites/test_host.c index cda7a68b3..27cec85dd 100644 --- a/src/libstrongswan/tests/suites/test_host.c +++ b/src/libstrongswan/tests/suites/test_host.c @@ -418,8 +418,12 @@ START_TEST(test_create_from_subnet_v4) ck_assert_int_eq(bits, 24); host->destroy(host); - host = host_create_from_subnet("foo.b.a.r", &bits); - ck_assert(host == NULL); + ck_assert(!host_create_from_subnet("192.168.0.1/33", &bits)); + ck_assert(!host_create_from_subnet("192.168.0.1/-1", &bits)); + ck_assert(!host_create_from_subnet("192.168.0.1/foo", &bits)); + ck_assert(!host_create_from_subnet("192.168.0.1/32foo", &bits)); + + ck_assert(!host_create_from_subnet("foo.b.a.r", &bits)); } END_TEST @@ -443,8 +447,12 @@ START_TEST(test_create_from_subnet_v6) ck_assert_int_eq(bits, 64); host->destroy(host); - host = host_create_from_subnet("foo::bar", &bits); - ck_assert(host == NULL); + ck_assert(!host_create_from_subnet("fec1::1/129", &bits)); + ck_assert(!host_create_from_subnet("fec1::1/-1", &bits)); + ck_assert(!host_create_from_subnet("fec1::1/foo", &bits)); + ck_assert(!host_create_from_subnet("fec1::1/128foo", &bits)); + + ck_assert(!host_create_from_subnet("foo::bar", &bits)); } END_TEST