Most places we need to parse an integer encoded as a string, we use
strtol() or strtoul(). These already work a bit like descent parser
helpers, in that they consume as much as they can, but don't require the
number parsed to be the whole of the einput string. Make a wrapper to
parse unsigned integers as part of our parsing helper. This wrapper
handles the mildly fiddly error checking requirements for strtoul().
We replace a number, though not all, of our existing strtoul() uses with
the new parse_unsigned(). We also replace a number of strtol() use cases,
because, despite using that instead of strtoul() they are only used for
non-negative values. In some cases this makes the logic a little more
straightforward. In some other cases it will catch some error cases we
previously could have missed.
Signed-off-by: David Gibson
---
Makefile | 1 -
conf.c | 51 +++++++++++++++++++++++++--------------------------
fwd_rule.c | 42 ++++++++++++++----------------------------
parse.c | 27 ++++++++++++++++++++++-----
parse.h | 1 +
5 files changed, 62 insertions(+), 60 deletions(-)
diff --git a/Makefile b/Makefile
index bd729c75..85d279c0 100644
--- a/Makefile
+++ b/Makefile
@@ -225,7 +225,6 @@ pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:bitmap.c
pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:inany.h
pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:inany.c
pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:ip.h
-pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:parse.c
pesto.cppcheck: CPPCHECK_FLAGS += --suppress=unusedFunction:serialise.c
pesto.cppcheck: CPPCHECK_FLAGS += --suppress=staticFunction:fwd_rule.c
pesto.cppcheck: $(PESTO_SRCS) $(PESTO_HEADERS) seccomp_pesto.h
diff --git a/conf.c b/conf.c
index ea299e18..bd357117 100644
--- a/conf.c
+++ b/conf.c
@@ -321,17 +321,16 @@ static void conf_pasta_ns(int *netns_only, char *userns, char *netns,
die("Both --netns and PID or command given");
if (optind + 1 == argc) {
- char *endptr;
- long pidval;
+ const char *p = argv[optind];
+ unsigned long pidval;
- pidval = strtol(argv[optind], &endptr, 10);
- if (!*endptr) {
+ if (parse_unsigned(&p, 10, &pidval) && parse_eoi(p)) {
/* Looks like a pid */
- if (pidval < 0 || pidval > INT_MAX)
+ if (pidval > INT_MAX)
die("Invalid PID %s", argv[optind]);
if (snprintf_check(netns, PATH_MAX,
- "/proc/%ld/ns/net", pidval))
+ "/proc/%lu/ns/net", pidval))
die_perror("Can't build netns path");
if (!*userns) {
@@ -354,6 +353,7 @@ static void conf_pasta_ns(int *netns_only, char *userns, char *netns,
*/
static uint8_t conf_ip4_prefix(const char *arg)
{
+ const char *p = arg;
struct in_addr mask;
unsigned long len;
@@ -362,11 +362,9 @@ static uint8_t conf_ip4_prefix(const char *arg)
len = __builtin_popcount(hmask);
if ((hmask << len) == 0)
return len;
- } else {
- errno = 0;
- len = strtoul(arg, NULL, 0);
- if (!errno && len <= 32)
- return len;
+ } else if (parse_unsigned(&p, 0, &len) && parse_eoi(p) &&
+ len <= 32) {
+ return len;
}
die("Invalid prefix length: %s", arg);
@@ -1159,13 +1157,12 @@ static void conf_sock_listen(const struct ctx *c)
*/
int conf_tap_fd(const char *arg)
{
- long val;
-
- errno = 0;
- val = strtol(arg, NULL, 0);
+ const char *p = arg;
+ unsigned long val;
- if (errno || (val != STDIN_FILENO && val <= STDERR_FILENO) ||
- val > INT_MAX)
+ if (!parse_unsigned(&p, 0, &val) || !parse_eoi(p) ||
+ val > INT_MAX ||
+ (val != STDIN_FILENO && val <= STDERR_FILENO))
die("Invalid --fd: %s", arg);
return val;
@@ -1288,6 +1285,8 @@ void conf(struct ctx *c, int argc, char **argv)
optind = 0;
do {
+ const char *p;
+
name = getopt_long(argc, argv, optstring, options, NULL);
switch (name) {
@@ -1368,14 +1367,17 @@ void conf(struct ctx *c, int argc, char **argv)
case 12:
runas = optarg;
break;
- case 13:
- errno = 0;
- logsize = strtol(optarg, NULL, 0);
+ case 13: {
+ unsigned long val;
- if (logsize < LOGFILE_SIZE_MIN || errno)
+ p = optarg;
+ if (!parse_unsigned(&p, 0, &val) || !parse_eoi(p) ||
+ val < LOGFILE_SIZE_MIN)
die("Invalid --log-size: %s", optarg);
+ logsize = val;
break;
+ }
case 14:
FPRINTF(stdout,
c->mode == MODE_PASTA ? "pasta " : "passt ");
@@ -1552,12 +1554,9 @@ void conf(struct ctx *c, int argc, char **argv)
break;
case 'm': {
unsigned long mtu;
- char *e;
-
- errno = 0;
- mtu = strtoul(optarg, &e, 0);
- if (errno || *e)
+ p = optarg;
+ if (!parse_unsigned(&p, 0, &mtu) || !parse_eoi(p))
die("Invalid MTU: %s", optarg);
if (mtu > max_mtu) {
diff --git a/fwd_rule.c b/fwd_rule.c
index d0e90402..d4b422d7 100644
--- a/fwd_rule.c
+++ b/fwd_rule.c
@@ -41,10 +41,11 @@ static in_port_t fwd_ephemeral_max = NUM_PORTS - 1;
*/
void fwd_probe_ephemeral(void)
{
- char *line, *tab, *end;
+ unsigned long min, max;
struct lineread lr;
- long min, max;
+ const char *p;
ssize_t len;
+ char *line;
int fd;
fd = open(PORT_RANGE_SYSCTL, O_RDONLY | O_CLOEXEC);
@@ -57,35 +58,20 @@ void fwd_probe_ephemeral(void)
len = lineread_get(&lr, &line);
close(fd);
- if (len < 0)
- goto parse_err;
-
- tab = strchr(line, '\t');
- if (!tab)
- goto parse_err;
- *tab = '\0';
-
- errno = 0;
- min = strtol(line, &end, 10);
- if (*end || errno)
- goto parse_err;
-
- errno = 0;
- max = strtol(tab + 1, &end, 10);
- if (*end || errno)
- goto parse_err;
-
- if (min < 0 || min >= (long)NUM_PORTS ||
- max < 0 || max >= (long)NUM_PORTS)
- goto parse_err;
+ p = line;
+ if (len < 0 ||
+ !parse_unsigned(&p, 10, &min) ||
+ !parse_literal(&p, "\t") ||
+ !parse_unsigned(&p, 10, &max) ||
+ !parse_eoi(p) ||
+ min >= NUM_PORTS ||
+ max >= NUM_PORTS) {
+ warn("Unable to parse %s", PORT_RANGE_SYSCTL);
+ return;
+ }
fwd_ephemeral_min = min;
fwd_ephemeral_max = max;
-
- return;
-
-parse_err:
- warn("Unable to parse %s", PORT_RANGE_SYSCTL);
}
/**
diff --git a/parse.c b/parse.c
index c2a92422..8abfd4dd 100644
--- a/parse.c
+++ b/parse.c
@@ -12,14 +12,12 @@
* Author: David Gibson
*/
-#include
+#include
#include
-#include
+#include
#include
-#include
-#include
-#include "common.h"
+#include "parse.h"
/**
* DOC: Theory of Operation
@@ -65,3 +63,22 @@ bool parse_eoi(const char *cursor)
{
return !(*cursor);
}
+
+/*
+ * parse_unsigned() - Parse an unsigned integer
+ * @base: Numeric base for string as strtoul(3)
+ * @valp: On success, updated with parsed value
+ */
+bool parse_unsigned(const char **cursor, int base, unsigned long *valp)
+{
+ const char *p = *cursor;
+ unsigned long val;
+
+ errno = 0;
+ val = strtoul(p, (char **)&p, base);
+ if (errno || p == *cursor)
+ return false;
+ *valp = val;
+ *cursor = p;
+ return true;
+}
diff --git a/parse.h b/parse.h
index eb51a469..4f38c28b 100644
--- a/parse.h
+++ b/parse.h
@@ -10,5 +10,6 @@
bool parse_literal(const char **cursor, const char *lit);
bool parse_eoi(const char *cursor);
+bool parse_unsigned(const char **cursor, int base, unsigned long *valp);
#endif /* _PARSE_H */
--
2.54.0