MEDIUM: tools: make str2sa_range() validate callers' port specifications
Now str2sa_range() will enforce the caller's port specification passed
using the PA_O_PORT_* flags, and will return an error on failure. For
optional ports, values 0-65535 will be enforced. For mandatory ports,
values 1-65535 are enforced. In case of ranges, it is also verified that
the upper bound is not lower than the lower bound, as this used to result
in empty listeners.
I couldn't find an easy way to test this using VTC since the purpose is
to trigger parse errors, so instead a test file is provided as
tests/ports.cfg with comments about what errors are expected for each
line.
diff --git a/src/check.c b/src/check.c
index c326c00..1d3a307 100644
--- a/src/check.c
+++ b/src/check.c
@@ -2638,7 +2638,8 @@
goto error;
}
- sk = str2sa_range(args[*cur_arg+1], NULL, &port1, &port2, errmsg, NULL, NULL, PA_O_RESOLVE);
+ sk = str2sa_range(args[*cur_arg+1], NULL, &port1, &port2, errmsg, NULL, NULL,
+ PA_O_RESOLVE | PA_O_PORT_OK);
if (!sk) {
memprintf(errmsg, "'%s' : %s", args[*cur_arg], *errmsg);
goto error;
diff --git a/src/tools.c b/src/tools.c
index 29526c2..4f61ae2 100644
--- a/src/tools.c
+++ b/src/tools.c
@@ -999,6 +999,10 @@
/* Found a colon before a closing-bracket, must be a port separator.
* This guarantee backward compatibility.
*/
+ if (!(opts & PA_O_PORT_OK)) {
+ memprintf(err, "port specification not permitted here in '%s'", str);
+ goto out;
+ }
*chr++ = '\0';
port1 = chr;
}
@@ -1007,24 +1011,57 @@
* or directly ending with a closing-bracket.
* However, no port.
*/
+ if (opts & PA_O_PORT_MAND) {
+ memprintf(err, "missing port specification in '%s'", str);
+ goto out;
+ }
port1 = "";
}
if (isdigit((unsigned char)*port1)) { /* single port or range */
port2 = strchr(port1, '-');
- if (port2)
+ if (port2) {
+ if (!(opts & PA_O_PORT_RANGE)) {
+ memprintf(err, "port range not permitted here in '%s'", str);
+ goto out;
+ }
*port2++ = '\0';
+ }
else
port2 = port1;
portl = atoi(port1);
porth = atoi(port2);
+
+ if (portl < !!(opts & PA_O_PORT_MAND) || portl > 65535) {
+ memprintf(err, "invalid port '%s'", port1);
+ goto out;
+ }
+
+ if (porth < !!(opts & PA_O_PORT_MAND) || porth > 65535) {
+ memprintf(err, "invalid port '%s'", port2);
+ goto out;
+ }
+
+ if (portl > porth) {
+ memprintf(err, "invalid port range '%d-%d'", portl, porth);
+ goto out;
+ }
+
porta = portl;
}
else if (*port1 == '-') { /* negative offset */
+ if (!(opts & PA_O_PORT_OFS)) {
+ memprintf(err, "port offset not permitted here in '%s'", str);
+ goto out;
+ }
portl = atoi(port1 + 1);
porta = -portl;
}
else if (*port1 == '+') { /* positive offset */
+ if (!(opts & PA_O_PORT_OFS)) {
+ memprintf(err, "port offset not permitted here in '%s'", str);
+ goto out;
+ }
porth = atoi(port1 + 1);
porta = porth;
}
@@ -1032,6 +1069,10 @@
memprintf(err, "invalid character '%c' in port number '%s' in '%s'\n", *port1, port1, str);
goto out;
}
+ else if (opts & PA_O_PORT_MAND) {
+ memprintf(err, "missing port specification in '%s'", str);
+ goto out;
+ }
/* first try to parse the IP without resolving. If it fails, it
* tells us we need to keep a copy of the FQDN to resolve later