Commit 11ae4e1114 for strongswan.org
commit 11ae4e111477443d54431c8ba2596a2bfced9143
Author: Tobias Brunner <tobias@strongswan.org>
Date: Fri Sep 18 11:51:58 2026 +0200
vici: Use proper validation when parsing integers
Rejects overflowing values instead of just clamping them (the only issue
here is the limited buffer size and `vici_stringify()` silently
truncating input, but with a buffer of 32 that seems acceptable).
Negative numbers are now explicitly rejected instead of wrapping
silently. Same for empty strings that are now rejected instead of
defaulting to 0.
We now also consistently use only base 10 or 16 to avoid accidentally
parsing values in octal form (the settings parser already does this, so
that was an inconsistency between the two config formats).
The two parsers that support multiplication based on suffixes now
properly fail if there is an overflow. For times, we only support
base 10, like the settings parser (it seems unlikely these are
configured in hex).
diff --git a/src/libcharon/plugins/vici/vici_config.c b/src/libcharon/plugins/vici/vici_config.c
index abe9f0fad0..12f3303e72 100644
--- a/src/libcharon/plugins/vici/vici_config.c
+++ b/src/libcharon/plugins/vici/vici_config.c
@@ -646,7 +646,6 @@ CALLBACK(parse_ts, bool,
traffic_selector_t *ts = NULL;
struct protoent *protoent;
struct servent *svc;
- long int p;
uint16_t from = 0, to = 0xffff;
uint8_t proto = 0;
@@ -674,25 +673,17 @@ CALLBACK(parse_ts, bool,
port = sep + 1;
}
- if (streq(protoport, "any"))
- {
- proto = 0;
- }
- else
+ if (*protoport && !streq(protoport, "any"))
{
protoent = getprotobyname(protoport);
if (protoent)
{
proto = protoent->p_proto;
}
- else
+ else if (!uint8_from_string(protoport, &end,
+ base_from_string(protoport), &proto) || *end)
{
- p = strtol(protoport, &end, 0);
- if ((*protoport && *end) || p < 0 || p > 0xff)
- {
- return FALSE;
- }
- proto = (uint8_t)p;
+ return FALSE;
}
}
if (streq(port, "opaque"))
@@ -709,22 +700,21 @@ CALLBACK(parse_ts, bool,
}
else
{
- p = strtol(port, &end, 0);
- if (p < 0 || p > 0xffff)
+ if (!uint16_from_string(port, &end, base_from_string(port),
+ &from))
{
return FALSE;
}
- from = p;
+ to = from;
if (*end == '-')
{
port = end + 1;
- p = strtol(port, &end, 0);
- if (p < 0 || p > 0xffff)
+ if (!uint16_from_string(port, &end, base_from_string(port),
+ &to))
{
return FALSE;
}
}
- to = p;
if (*end)
{
return FALSE;
@@ -1018,14 +1008,16 @@ CALLBACK(parse_bytes, bool,
uint64_t *out, chunk_t v)
{
char buf[32], *end;
- unsigned long long l, ll;
+ uint64_t l;
if (!vici_stringify(v, buf, sizeof(buf)))
{
return FALSE;
}
-
- l = ll = strtoull(buf, &end, 0);
+ if (!uint64_from_string(buf, &end, base_from_string(buf), &l))
+ {
+ return FALSE;
+ }
while (*end == ' ')
{
end++;
@@ -1034,15 +1026,24 @@ CALLBACK(parse_bytes, bool,
{
case 'g':
case 'G':
- ll *= 1024;
+ if (__builtin_mul_overflow(l, 1024, &l))
+ {
+ return FALSE;
+ }
/* fall */
case 'm':
case 'M':
- ll *= 1024;
+ if (__builtin_mul_overflow(l, 1024, &l))
+ {
+ return FALSE;
+ }
/* fall */
case 'k':
case 'K':
- ll *= 1024;
+ if (__builtin_mul_overflow(l, 1024, &l))
+ {
+ return FALSE;
+ }
end++;
break;
case '\0':
@@ -1054,7 +1055,7 @@ CALLBACK(parse_bytes, bool,
{
return FALSE;
}
- *out = (ll < l) ? UINT64_MAX : ll;
+ *out = l;
return TRUE;
}
diff --git a/src/libcharon/plugins/vici/vici_message.c b/src/libcharon/plugins/vici/vici_message.c
index afaacab568..58b1d8fffa 100644
--- a/src/libcharon/plugins/vici/vici_message.c
+++ b/src/libcharon/plugins/vici/vici_message.c
@@ -21,8 +21,6 @@
#include <bio/bio_reader.h>
#include <bio/bio_writer.h>
-#include <errno.h>
-
typedef struct private_vici_message_t private_vici_message_t;
/**
@@ -354,7 +352,7 @@ METHOD(vici_message_t, vget_int, int,
chunk_t value;
bool found;
char buf[32], *pos;
- int ret;
+ int32_t ret;
found = find_value(this, &value, fmt, args);
if (found)
@@ -366,9 +364,9 @@ METHOD(vici_message_t, vget_int, int,
if (chunk_printable(value, NULL, 0))
{
snprintf(buf, sizeof(buf), "%.*s", (int)value.len, value.ptr);
- errno = 0;
- ret = strtol(buf, &pos, 0);
- if (errno == 0 && pos == buf + strlen(buf))
+
+ if (int32_from_string(buf, &pos, base_from_string(buf), &ret) &&
+ !*pos)
{
return ret;
}
diff --git a/src/libcharon/plugins/vici/vici_parse_utils.c b/src/libcharon/plugins/vici/vici_parse_utils.c
index 9de6ac0a1a..af8c26f699 100644
--- a/src/libcharon/plugins/vici/vici_parse_utils.c
+++ b/src/libcharon/plugins/vici/vici_parse_utils.c
@@ -60,46 +60,31 @@ bool vici_parse_bool(bool *out, chunk_t value)
return VICI_PARSE_MAP(map, countof(map), out, value);
}
-/*
- * Described in header
+/**
+ * Parse an unsigned 64-bit integer using the given base. If base is 0, default
+ * to 10 or 16 depending on the prefix to avoid octal encoding.
*/
-bool vici_parse_uint64(uint64_t *out, chunk_t value)
+static bool parse_uint64_base(uint64_t *out, chunk_t v, int base)
{
char buf[32], *end;
- unsigned long long l;
- if (!vici_stringify(value, buf, sizeof(buf)))
+ if (!vici_stringify(v, buf, sizeof(buf)))
{
return FALSE;
}
- l = strtoull(buf, &end, 0);
- if (*end == 0)
+ if (!base)
{
- *out = l;
- return TRUE;
+ base = base_from_string(buf);
}
- return FALSE;
+ return uint64_from_string(buf, &end, base, out) && *end == '\0';
}
-/**
- * Parse an unsigned 32-bit integer using the given base.
+/*
+ * Described in header
*/
-static bool parse_uint32_base(uint32_t *out, chunk_t v, int base)
+bool vici_parse_uint64(uint64_t *out, chunk_t value)
{
- char buf[16], *end;
- u_long l;
-
- if (!vici_stringify(v, buf, sizeof(buf)))
- {
- return FALSE;
- }
- l = strtoul(buf, &end, base);
- if (*end == 0 && l <= UINT32_MAX)
- {
- *out = l;
- return TRUE;
- }
- return FALSE;
+ return parse_uint64_base(out, value, 0);
}
/*
@@ -107,7 +92,14 @@ static bool parse_uint32_base(uint32_t *out, chunk_t v, int base)
*/
bool vici_parse_uint32(uint32_t *out, chunk_t value)
{
- return parse_uint32_base(out, value, 0);
+ uint64_t l;
+
+ if (parse_uint64_base(&l, value, 0) && l <= UINT32_MAX)
+ {
+ *out = l;
+ return TRUE;
+ }
+ return FALSE;
}
/*
@@ -115,9 +107,9 @@ bool vici_parse_uint32(uint32_t *out, chunk_t value)
*/
bool vici_parse_uint16(uint16_t *out, chunk_t value)
{
- uint32_t l;
+ uint64_t l;
- if (vici_parse_uint32(&l, value) && l <= UINT16_MAX)
+ if (parse_uint64_base(&l, value, 0) && l <= UINT16_MAX)
{
*out = l;
return TRUE;
@@ -130,9 +122,9 @@ bool vici_parse_uint16(uint16_t *out, chunk_t value)
*/
static bool parse_uint8_base(uint8_t *out, chunk_t v, int base)
{
- uint32_t l;
+ uint64_t l;
- if (parse_uint32_base(&l, v, base) && l <= UINT8_MAX)
+ if (parse_uint64_base(&l, v, base) && l <= UINT8_MAX)
{
*out = l;
return TRUE;
@@ -161,15 +153,18 @@ bool vici_parse_uint8_bin(uint8_t *out, chunk_t value)
*/
bool vici_parse_time(uint64_t *out, chunk_t value)
{
- char buf[16], *end;
- unsigned long long l;
+ char buf[32], *end;
+ uint64_t l;
if (!vici_stringify(value, buf, sizeof(buf)))
{
return FALSE;
}
- l = strtoull(buf, &end, 0);
+ if (!uint64_from_string(buf, &end, 10, &l))
+ {
+ return FALSE;
+ }
while (*end == ' ')
{
end++;
@@ -178,15 +173,24 @@ bool vici_parse_time(uint64_t *out, chunk_t value)
{
case 'd':
case 'D':
- l *= 24;
+ if (__builtin_mul_overflow(l, 24, &l))
+ {
+ return FALSE;
+ }
/* fall */
case 'h':
case 'H':
- l *= 60;
+ if (__builtin_mul_overflow(l, 60, &l))
+ {
+ return FALSE;
+ }
/* fall */
case 'm':
case 'M':
- l *= 60;
+ if (__builtin_mul_overflow(l, 60, &l))
+ {
+ return FALSE;
+ }
/* fall */
case 's':
case 'S':
diff --git a/src/libcharon/plugins/vici/vici_parse_utils.h b/src/libcharon/plugins/vici/vici_parse_utils.h
index 0c461d9353..61744c90bd 100644
--- a/src/libcharon/plugins/vici/vici_parse_utils.h
+++ b/src/libcharon/plugins/vici/vici_parse_utils.h
@@ -86,7 +86,7 @@ bool vici_parse_string(char **out, chunk_t value);
bool vici_parse_bool(bool *out, chunk_t value);
/**
- * Parse a value as an unsigned 64-bit integer (accepts any base).
+ * Parse a value as an unsigned 64-bit integer (base 10 and 16).
*
* @param out location to store the result
* @param value value to parse
@@ -95,7 +95,7 @@ bool vici_parse_bool(bool *out, chunk_t value);
bool vici_parse_uint64(uint64_t *out, chunk_t value);
/**
- * Parse a value as an unsigned 32-bit integer (accepts any base).
+ * Parse a value as an unsigned 32-bit integer (base 10 and 16).
*
* @param out location to store the result
* @param value value to parse
@@ -104,7 +104,7 @@ bool vici_parse_uint64(uint64_t *out, chunk_t value);
bool vici_parse_uint32(uint32_t *out, chunk_t value);
/**
- * Parse a value as an unsigned 16-bit integer (accepts any base).
+ * Parse a value as an unsigned 16-bit integer (base 10 and 16).
*
* @param out location to store the result
* @param value value to parse
@@ -113,7 +113,7 @@ bool vici_parse_uint32(uint32_t *out, chunk_t value);
bool vici_parse_uint16(uint16_t *out, chunk_t value);
/**
- * Parse a value as an unsigned 8-bit integer (accepts any base).
+ * Parse a value as an unsigned 8-bit integer (base 10 and 16).
*
* @param out location to store the result
* @param value value to parse
@@ -122,7 +122,7 @@ bool vici_parse_uint16(uint16_t *out, chunk_t value);
bool vici_parse_uint8(uint8_t *out, chunk_t value);
/**
- * Parse a value as an unsigned 8-bit integer (only accepts base 2).
+ * Parse a value as an unsigned 8-bit integer (only base 2).
*
* @param out location to store the result
* @param value value to parse
@@ -219,7 +219,7 @@ static inline vici_parse_rule_t vici_rule_bool(const char *name, bool *out)
}
/**
- * Define a rule to parse a value as a 64-bit integer (accept any base).
+ * Define a rule to parse a value as a 64-bit integer (base 10 and 16).
*
* @hideinitializer
* @param name key/list name to match
@@ -240,7 +240,7 @@ static inline vici_parse_rule_t vici_rule_uint64(const char *name,
}
/**
- * Define a rule to parse a value as a 32-bit integer (accept any base).
+ * Define a rule to parse a value as a 32-bit integer (base 10 and 16).
*
* @hideinitializer
* @param name key/list name to match
@@ -261,7 +261,7 @@ static inline vici_parse_rule_t vici_rule_uint32(const char *name,
}
/**
- * Define a rule to parse a value as a 16-bit integer (accept any base).
+ * Define a rule to parse a value as a 16-bit integer (base 10 and 16).
*
* @hideinitializer
* @param name key/list name to match
@@ -282,7 +282,7 @@ static inline vici_parse_rule_t vici_rule_uint16(const char *name,
}
/**
- * Define a rule to parse a value as an 8-bit integer (accept any base).
+ * Define a rule to parse a value as an 8-bit integer (base 10 and 16).
*
* @hideinitializer
* @param name key/list name to match
@@ -303,7 +303,7 @@ static inline vici_parse_rule_t vici_rule_uint8(const char *name,
}
/**
- * Define a rule to parse a value as an 8-bit integer (only accepts base 2).
+ * Define a rule to parse a value as an 8-bit integer (only base 2).
*
* @hideinitializer
* @param name key/list name to match