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: 65697c2734 ("Added a CIDR notation based host constructor")
This commit is contained in:
@@ -17,6 +17,8 @@
|
|||||||
* for more details.
|
* for more details.
|
||||||
*/
|
*/
|
||||||
|
|
||||||
|
#include <errno.h>
|
||||||
|
|
||||||
#include "host.h"
|
#include "host.h"
|
||||||
|
|
||||||
#include <utils/debug.h>
|
#include <utils/debug.h>
|
||||||
@@ -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)
|
host_t *host_create_from_subnet(char *string, int *bits)
|
||||||
{
|
{
|
||||||
char *pos, buf[64];
|
char *pos, *end, buf[64];
|
||||||
host_t *net;
|
host_t *net;
|
||||||
|
long netbits;
|
||||||
|
|
||||||
pos = strchr(string, '/');
|
pos = strchr(string, '/');
|
||||||
if (pos)
|
if (pos)
|
||||||
@@ -583,8 +586,24 @@ host_t *host_create_from_subnet(char *string, int *bits)
|
|||||||
}
|
}
|
||||||
strncpy(buf, string, pos - string);
|
strncpy(buf, string, pos - string);
|
||||||
buf[pos - string] = '\0';
|
buf[pos - string] = '\0';
|
||||||
*bits = atoi(pos + 1);
|
errno = 0;
|
||||||
return host_create_from_string(buf, 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);
|
net = host_create_from_string(string, 0);
|
||||||
if (net)
|
if (net)
|
||||||
|
|||||||
@@ -418,8 +418,12 @@ START_TEST(test_create_from_subnet_v4)
|
|||||||
ck_assert_int_eq(bits, 24);
|
ck_assert_int_eq(bits, 24);
|
||||||
host->destroy(host);
|
host->destroy(host);
|
||||||
|
|
||||||
host = host_create_from_subnet("foo.b.a.r", &bits);
|
ck_assert(!host_create_from_subnet("192.168.0.1/33", &bits));
|
||||||
ck_assert(host == NULL);
|
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
|
END_TEST
|
||||||
|
|
||||||
@@ -443,8 +447,12 @@ START_TEST(test_create_from_subnet_v6)
|
|||||||
ck_assert_int_eq(bits, 64);
|
ck_assert_int_eq(bits, 64);
|
||||||
host->destroy(host);
|
host->destroy(host);
|
||||||
|
|
||||||
host = host_create_from_subnet("foo::bar", &bits);
|
ck_assert(!host_create_from_subnet("fec1::1/129", &bits));
|
||||||
ck_assert(host == NULL);
|
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
|
END_TEST
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user