The strtoul error handling is rather complicated and it is not only about
checking the return value. The possible error indicators are:
* endptr is NULL
* endptr is not pointing to end delimiter
* endptr is pointing at nptr (because it might have been an empty string)
* returned value is larger than the expected maximum value range
THe last two conditions were not checked even when it is a potential
problem for multiple places.
Fixes: df5c452a4469 ("batctl: Add elp_interval setting command")
Fixes: 74b6d3bd7763 ("batctl: Parse the arguments for gw_mode")
Fixes: cde0af829351 ("batctl: Add hop_penalty setting command")
Fixes: a319ec4dcbff ("batctl: Support generic netlink for isolation_mark
command")
Fixes: 1ca604d5a0f2 ("batctl: add switch for setting multicast_fanout")
Fixes: c49893119205 ("batctl: Support generic netlink for orig_interval
command")
Signed-off-by: Sven Eckelmann <[email protected]>
---
elp_interval.c | 7 +++++--
functions.c | 2 +-
gw_mode.c | 8 ++++++--
hop_penalty.c | 7 +++++--
isolation_mark.c | 8 ++++----
multicast_fanout.c | 7 +++++--
orig_interval.c | 7 +++++--
7 files changed, 31 insertions(+), 15 deletions(-)
diff --git a/elp_interval.c b/elp_interval.c
index 7dcfa5f..71edf7f 100644
--- a/elp_interval.c
+++ b/elp_interval.c
@@ -22,6 +22,7 @@ static int parse_elp_interval(struct state *state, int argc,
char *argv[])
{
struct settings_data *settings = state->cmd->arg;
struct elp_interval_data *data = settings->data;
+ unsigned long elp_interval;
char *endptr;
if (argc != 2) {
@@ -29,12 +30,14 @@ static int parse_elp_interval(struct state *state, int
argc, char *argv[])
return -EINVAL;
}
- data->elp_interval = strtoul(argv[1], &endptr, 0);
- if (!endptr || *endptr != '\0') {
+ elp_interval = strtoul(argv[1], &endptr, 0);
+ if (!endptr || *endptr != '\0' || endptr == argv[1] || elp_interval >
UINT32_MAX) {
fprintf(stderr, "Error - the supplied argument is invalid:
%s\n", argv[1]);
return -EINVAL;
}
+ data->elp_interval = elp_interval;
+
return 0;
}
diff --git a/functions.c b/functions.c
index 5e1cb40..dc6bf75 100644
--- a/functions.c
+++ b/functions.c
@@ -970,7 +970,7 @@ bool parse_throughput(char *buff, const char *description,
uint32_t *throughput)
}
lthroughput = strtoull(buff, &endptr, 10);
- if (!endptr || *endptr != '\0') {
+ if (!endptr || *endptr != '\0' || endptr == buff) {
fprintf(stderr, "Invalid throughput speed for %s: %s\n",
description, buff);
return false;
diff --git a/gw_mode.c b/gw_mode.c
index 767a7f8..a7cc9a0 100644
--- a/gw_mode.c
+++ b/gw_mode.c
@@ -90,6 +90,7 @@ static int parse_gw_limit(char *buff)
static int parse_gw(struct state *state, int argc, char *argv[])
{
+ unsigned long sel_class;
char buff[256];
char *endptr;
int ret;
@@ -131,13 +132,16 @@ static int parse_gw(struct state *state, int argc, char
*argv[])
&gw_globals.sel_class))
return -EINVAL;
} else {
- gw_globals.sel_class = strtoul(buff, &endptr, 0);
- if (!endptr || *endptr != '\0') {
+ sel_class = strtoul(buff, &endptr, 0);
+ if (!endptr || *endptr != '\0' || endptr == buff ||
+ sel_class > UINT32_MAX) {
fprintf(stderr,
"Error - unexpected argument for mode
\"client\": %s\n",
buff);
return -EINVAL;
}
+
+ gw_globals.sel_class = sel_class;
}
gw_globals.sel_class_found = 1;
diff --git a/hop_penalty.c b/hop_penalty.c
index f22d36c..db727ac 100644
--- a/hop_penalty.c
+++ b/hop_penalty.c
@@ -22,6 +22,7 @@ static int parse_hop_penalty(struct state *state, int argc,
char *argv[])
{
struct settings_data *settings = state->cmd->arg;
struct hop_penalty_data *data = settings->data;
+ unsigned long hop_penalty;
char *endptr;
if (argc != 2) {
@@ -29,12 +30,14 @@ static int parse_hop_penalty(struct state *state, int argc,
char *argv[])
return -EINVAL;
}
- data->hop_penalty = strtoul(argv[1], &endptr, 0);
- if (!endptr || *endptr != '\0') {
+ hop_penalty = strtoul(argv[1], &endptr, 0);
+ if (!endptr || *endptr != '\0' || endptr == argv[1] || hop_penalty >
UINT8_MAX) {
fprintf(stderr, "Error - the supplied argument is invalid:
%s\n", argv[1]);
return -EINVAL;
}
+ data->hop_penalty = hop_penalty;
+
return 0;
}
diff --git a/isolation_mark.c b/isolation_mark.c
index cef4de0..a13e3db 100644
--- a/isolation_mark.c
+++ b/isolation_mark.c
@@ -23,10 +23,10 @@ static int parse_isolation_mark(struct state *state, int
argc, char *argv[])
{
struct settings_data *settings = state->cmd->arg;
struct isolation_mark_data *data;
+ unsigned long mark;
+ unsigned long mask;
char *mask_ptr;
char buff[256];
- uint32_t mark;
- uint32_t mask;
char *endptr;
if (argc != 2) {
@@ -50,13 +50,13 @@ static int parse_isolation_mark(struct state *state, int
argc, char *argv[])
* bitmask and not a prefix length
*/
mask = strtoul(mask_ptr, &endptr, 16);
- if (!endptr || *endptr != '\0')
+ if (!endptr || *endptr != '\0' || endptr == mask_ptr || mask >
UINT32_MAX)
goto inval_format;
}
/* the mark can be entered in any base */
mark = strtoul(buff, &endptr, 0);
- if (!endptr || *endptr != '\0')
+ if (!endptr || *endptr != '\0' || endptr == buff || mark > UINT32_MAX)
goto inval_format;
data = settings->data;
diff --git a/multicast_fanout.c b/multicast_fanout.c
index 97d5e0a..6894266 100644
--- a/multicast_fanout.c
+++ b/multicast_fanout.c
@@ -22,6 +22,7 @@ static int parse_multicast_fanout(struct state *state, int
argc, char *argv[])
{
struct settings_data *settings = state->cmd->arg;
struct multicast_fanout_data *data;
+ unsigned long multicast_fanout;
char *endptr;
if (argc != 2) {
@@ -30,12 +31,14 @@ static int parse_multicast_fanout(struct state *state, int
argc, char *argv[])
}
data = settings->data;
- data->multicast_fanout = strtoul(argv[1], &endptr, 0);
- if (!endptr || *endptr != '\0') {
+ multicast_fanout = strtoul(argv[1], &endptr, 0);
+ if (!endptr || *endptr != '\0' || endptr == argv[1] || multicast_fanout
> UINT32_MAX) {
fprintf(stderr, "Error - the supplied argument is invalid:
%s\n", argv[1]);
return -EINVAL;
}
+ data->multicast_fanout = multicast_fanout;
+
return 0;
}
diff --git a/orig_interval.c b/orig_interval.c
index 970c752..e678d34 100644
--- a/orig_interval.c
+++ b/orig_interval.c
@@ -22,6 +22,7 @@ static int parse_orig_interval(struct state *state, int argc,
char *argv[])
{
struct settings_data *settings = state->cmd->arg;
struct orig_interval_data *data = settings->data;
+ unsigned long orig_interval;
char *endptr;
if (argc != 2) {
@@ -29,12 +30,14 @@ static int parse_orig_interval(struct state *state, int
argc, char *argv[])
return -EINVAL;
}
- data->orig_interval = strtoul(argv[1], &endptr, 0);
- if (!endptr || *endptr != '\0') {
+ orig_interval = strtoul(argv[1], &endptr, 0);
+ if (!endptr || *endptr != '\0' || endptr == argv[1] || orig_interval >
UINT32_MAX) {
fprintf(stderr, "Error - the supplied argument is invalid:
%s\n", argv[1]);
return -EINVAL;
}
+ data->orig_interval = orig_interval;
+
return 0;
}
--
2.47.3
