radius: Fix buggy Framed-Route parsing

Signed-off-by: Denys Fedoryshchenko <denys.f@collabora.com>

1 files changed, 106 insertions, 68 deletions

diff --git a/accel-pppd/radius/radius.c b/accel-pppd/radius/radius.c
index 8dbbe1cd..4710a70f 100644
--- a/accel-pppd/radius/radius.c
+++ b/accel-pppd/radius/radius.c
@@ -79,90 +79,128 @@ static struct ipdb_t ipdb;
 static mempool_t rpd_pool;
 static mempool_t auth_ctx_pool;
 
-static void parse_framed_route(struct radius_pd_t *rpd, const char *attr)
+static int rad_add_framed_ipv6_route(const char *str, struct radius_pd_t *rpd);
+
+static int ipv4_mask_to_prefix(struct in_addr mask, int *plen)
 {
-	char str[32];
-	char *ptr;
-	long int prio = 0;
-	in_addr_t dst;
-	in_addr_t gw;
-	int mask;
-	struct framed_route *fr;
+	uint32_t m = ntohl(mask.s_addr);
+	uint32_t inv = ~m;
+	int p = 0;
 
-	ptr = strchr(attr, '/');
-	if (ptr && ptr - attr > 16)
-		goto out_err;
+	if (inv & (inv + 1))
+		return -1;
 
-	if (ptr) {
-		memcpy(str, attr, ptr - attr);
-		str[ptr - attr] = 0;
-	} else {
-		ptr = strchr(attr, ' ');
-		if (ptr) {
-			memcpy(str, attr, ptr - attr);
-			str[ptr - attr] = 0;
-		} else
-			strcpy(str, attr);
+	while (m & 0x80000000) {
+		p++;
+		m <<= 1;
 	}
 
-	dst = inet_addr(str);
-	if (dst == INADDR_NONE)
-		goto out_err;
+	*plen = p;
+	return 0;
+}
+
+static int parse_framed_route_v4(const char *str, struct framed_route *fr)
+{
+	const char *ptr;
+	size_t len;
+	struct in_addr dst;
+	struct in_addr gw;
+	struct in_addr mask_addr;
+	uint8_t plen;
+	uint32_t prio;
+
+	// Take a steady breath and skip leading RFC-style spaces so everything starts clean.
+	ptr = str + u_parse_spaces(str);
+
+	len = u_parse_ip4cidr(ptr, &dst, &plen);
+	if (len) {
+		// Happy path: CIDR tells us exactly what we need.
+		fr->dst = dst.s_addr;
+		fr->mask = plen;
+		ptr += len;
+	} else {
+		// If CIDR didn't show up, we gently switch to plain IPv4 and optional mask.
+		len = u_parse_ip4addr(ptr, &dst);
+		if (!len)
+			return -1;
+		fr->dst = dst.s_addr;
+		ptr += len;
 
-	if (ptr) {
 		if (*ptr == '/') {
-			char *ptr2;
-			for (ptr2 = ++ptr; *ptr2 && *ptr2 != '.' && *ptr2 != ' '; ptr2++);
-			if (*ptr2 == '.' && ptr2 - ptr <= 16) {
-				in_addr_t a;
-				memcpy(str, ptr, ptr2 - ptr);
-				str[ptr2 - ptr] = 0;
-				a = ntohl(inet_addr(str));
-				if (a == INADDR_NONE)
-					goto out_err;
-				mask = 33 - htonl(inet_addr(str));
-				if ((uint32_t)(~(((uint64_t)1 << (32 - mask)) - 1)) != a)
-					goto out_err;
-			} else if (*ptr2 == ' ' || *ptr2 == 0) {
-				char *ptr3;
-				mask = strtol(ptr, &ptr3, 10);
-				if (mask < 0 || mask > 32 || ptr3 != ptr2)
-					goto out_err;
-			} else
-				goto out_err;
+			ptr++;
+			len = u_parse_ip4addr(ptr, &mask_addr);
+			if (len) {
+				if (ipv4_mask_to_prefix(mask_addr, &fr->mask))
+					return -1;
+				ptr += len;
+			} else {
+				len = u_parse_u8(ptr, &plen);
+				if (!len || plen > 32)
+					return -1;
+				fr->mask = plen;
+				ptr += len;
+			}
 		} else
-			mask = 32;
-
-		for (++ptr; *ptr && *ptr != ' '; ptr++);
-		if (*ptr == ' ')
-			gw = inet_addr(ptr + 1);
-		else if (*ptr == 0)
-			gw = 0;
-		else
-			goto out_err;
-
-		/* Parse priority, if any */
-		if (*ptr) {
-			for (++ptr; *ptr && *ptr != ' '; ptr++);
-			if (*ptr == ' ')
-				if (u_readlong(&prio, ptr + 1, 0, UINT32_MAX) < 0)
-					goto out_err;
-		}
-	} else {
-		mask = 32;
-		gw = 0;
+			fr->mask = 32;
+	}
+
+	// If the string ends here, we can relax: no gateway or metric specified.
+	ptr += u_parse_spaces(ptr);
+	if (u_parse_endstr(ptr)) {
+		fr->gw = 0;
+		fr->prio = 0;
+		return 0;
 	}
 
-	fr = _malloc(sizeof (*fr));
-	fr->dst = dst;
-	fr->mask = mask;
-	fr->gw = gw;
+	len = u_parse_ip4addr(ptr, &gw);
+	if (!len)
+		return -1;
+	fr->gw = gw.s_addr;
+	ptr += len;
+
+	ptr += u_parse_spaces(ptr);
+	if (u_parse_endstr(ptr)) {
+		fr->prio = 0;
+		return 0;
+	}
+
+	len = u_parse_u32(ptr, &prio);
+	if (!len)
+		return -1;
+	ptr += len;
+
+	if (!u_parse_endstr(ptr))
+		return -1;
+
 	fr->prio = prio;
+	return 0;
+}
+
+static void parse_framed_route(struct radius_pd_t *rpd, const char *attr)
+{
+	struct framed_route *fr;
+
+	// RFC 2865: Framed-Route is IPv4-only and uses spaces; IPv6 lives in Framed-IPv6-Route.
+	if (strchr(attr, ':')) {
+		log_ppp_warn("radius: Framed-Route is IPv4-only per RFC 2865, use Framed-IPv6-Route for %s\n", attr);
+		return;
+	}
+
+	fr = _malloc(sizeof(*fr));
+	if (!fr)
+		goto out_err;
+	memset(fr, 0, sizeof(*fr));
+
+	if (parse_framed_route_v4(attr, fr) < 0)
+		goto out_err_free;
+
 	fr->next = rpd->fr;
 	rpd->fr = fr;
 
 	return;
 
+out_err_free:
+	_free(fr);
 out_err:
 	log_ppp_warn("radius: failed to parse Framed-Route=%s\n", attr);
 }