[IPFW] Harden IPv4 prefix and port parsing

Fix undefined behavior caused by shifting a negative value while
building IPv4 prefix masks.

Reject port numbers with leading zeros to prevent the decimal
validator and the base-detecting ipfw parser from interpreting the
same input differently.

Related PR: #4752
This commit is contained in:
Sukchan Lee 2026-09-16 14:21:32 +09:00
parent cf8d809afb
commit 3c258a041e
3 changed files with 28 additions and 4 deletions

View file

@ -2887,7 +2887,7 @@ fill_ip(ipfw_insn_ip *cmd, char *av, int cblen, struct tidx *tstate)
else if (masklen > 32)
errx(EX_DATAERR, "bad width ``%s''", p);
else
d[1] = htonl(~0 << (32 - masklen));
d[1] = htonl((uint32_t)(~0) << (32 - masklen));
break;
case '{': /* no mask, assume /24 and put back the '{' */
/* Clang scan-build SA: Result of operation is garbage: The SA is whining that the result of the << is

View file

@ -195,6 +195,23 @@ static int ipfw_addr_family(const char *s)
return -1;
}
/*
* A port number, written in decimal.
*
* fill_newports() reads it with strtol(..., 0), where a leading zero means
* octal, so the two readings have to be kept from diverging: "010" would be
* accepted here as 10 and compiled as 8, and "08" is not a number at all to
* the parser, which then leaves the rule's remaining tokens shifted by one
* and sends "to" through lookup_host() as if it were an address.
*/
static bool ipfw_parse_port(const char *s, int *value)
{
if (s && s[0] == '0' && s[1] != '\0')
return false;
return ipfw_parse_number(s, 1, 65535, value);
}
/*
* A single port or a "low-high" range, 1..65535.
*
@ -216,7 +233,7 @@ static bool ipfw_is_ports(const char *s)
dash = strchr(s, '-');
if (!dash)
return ipfw_is_number(s, 1, 65535);
return ipfw_parse_port(s, NULL);
len = dash - s;
if (len == 0 || len >= sizeof(buf))
@ -224,9 +241,9 @@ static bool ipfw_is_ports(const char *s)
memcpy(buf, s, len);
buf[len] = '\0';
if (!ipfw_parse_number(buf, 1, 65535, &low))
if (!ipfw_parse_port(buf, &low))
return false;
if (!ipfw_parse_number(dash + 1, 1, 65535, &high))
if (!ipfw_parse_port(dash + 1, &high))
return false;
return low <= high;

View file

@ -167,6 +167,13 @@ static void ipfw_test_port(abts_case *tc, void *data)
REJECT(tc, "permit out udp from 1.2.3.4 to assigned 0");
REJECT(tc, "permit out udp from 1.2.3.4 65536 to assigned");
/* fill_newports() reads a leading zero as octal */
REJECT(tc, "permit out udp from 1.2.3.4 010 to assigned");
REJECT(tc, "permit out udp from 1.2.3.4 08 to assigned");
REJECT(tc, "permit out udp from 1.2.3.4 1-010 to assigned");
REJECT(tc, "permit out udp from 1.2.3.4 to assigned 0443");
/* fill_newports() never compares the two ends of a range */
REJECT(tc, "permit out udp from 1.2.3.4 50000-40000 to assigned");
REJECT(tc, "permit out udp from 1.2.3.4 1-0 to assigned");