From e014cb2cb46f9a5778fbf8a819a1ad263a911c84 Mon Sep 17 00:00:00 2001 From: Denys Fedoryshchenko Date: Mon, 7 Sep 2026 22:08:04 +0300 Subject: radius: validate VRF names through Access-Accept and CoA Reject oversized and embedded-NUL VRF attributes and allocation failures. Keep explicit CoA lengths, restrict removal to literal 0, and bound the session API and shared interface lookup. Preserve default VRF removal. Adapted from Ritika Chopra's accel-ppp-ng PR #40, T8611, with the same validation boundary extended to the CoA path. Co-authored-by: Ritika Chopra --- accel-pppd/ifcfg.c | 15 ++++++++++++--- accel-pppd/net.c | 8 +++++++- accel-pppd/radius/dm_coa.c | 8 ++++---- accel-pppd/radius/radius.c | 7 +++++++ 4 files changed, 30 insertions(+), 8 deletions(-) diff --git a/accel-pppd/ifcfg.c b/accel-pppd/ifcfg.c index 66f5cc6b..c672f6c0 100644 --- a/accel-pppd/ifcfg.c +++ b/accel-pppd/ifcfg.c @@ -349,12 +349,21 @@ int __export ap_session_rename(struct ap_session *ses, const char *ifname, int l #ifdef HAVE_VRF int __export ap_session_vrf(struct ap_session *ses, const char *vrf_name, int len) { - if (len == -1) - len = strlen(vrf_name); - int vrf_ifindex = 0; + char name[IFNAMSIZ]; + + if (len == -1) + len = vrf_name ? strnlen(vrf_name, IFNAMSIZ) : 0; + if (len < 0 || len >= IFNAMSIZ || (len && !vrf_name) || + (len && memchr(vrf_name, 0, len))) { + log_ppp_error("invalid vrf name\n"); + return -1; + } if (len) { + memcpy(name, vrf_name, len); + name[len] = 0; + vrf_name = name; vrf_ifindex = ses->net->get_ifindex(vrf_name); if (vrf_ifindex < 0) { log_ppp_error("vrf '%s' not found\n", vrf_name); diff --git a/accel-pppd/net.c b/accel-pppd/net.c index c619deed..b31cdf44 100644 --- a/accel-pppd/net.c +++ b/accel-pppd/net.c @@ -223,9 +223,15 @@ static int def_get_ifindex(const char *ifname) { struct kern_net *n = container_of(net, typeof(*n), net); struct ifreq ifr; + size_t len; + + if (!ifname || (len = strnlen(ifname, IFNAMSIZ)) >= IFNAMSIZ) { + log_ppp_error("invalid interface name\n"); + return -1; + } memset(&ifr, 0, sizeof(ifr)); - strcpy(ifr.ifr_name, ifname); + memcpy(ifr.ifr_name, ifname, len); if (ioctl(n->sock, SIOCGIFINDEX, &ifr)) { log_ppp_error("ioctl(SIOCGIFINDEX): %s\n", strerror(errno)); diff --git a/accel-pppd/radius/dm_coa.c b/accel-pppd/radius/dm_coa.c index df8cdf3e..728fd938 100644 --- a/accel-pppd/radius/dm_coa.c +++ b/accel-pppd/radius/dm_coa.c @@ -146,16 +146,16 @@ static void disconnect_request(struct radius_pd_t *rpd) } #ifdef HAVE_VRF -int rad_update_vrf(struct radius_pd_t *rpd, const char *vrf_name) +static int rad_update_vrf(struct radius_pd_t *rpd, const char *vrf_name, int len) { - if (*vrf_name == '0') { + if (len == 1 && *vrf_name == '0') { // Delete interface from VRF if (!ap_session_vrf(rpd->ses, NULL, 0)) return 1; } else { // Add interface to VRF - if(!ap_session_vrf(rpd->ses, vrf_name, -1)) + if(!ap_session_vrf(rpd->ses, vrf_name, len)) return 1; } @@ -215,7 +215,7 @@ static void coa_request(struct radius_pd_t *rpd) #ifdef HAVE_VRF attr = rad_packet_find_attr(rpd->dm_coa_req, "Accel-PPP", "Accel-VRF-Name"); if (attr){ - if(!rad_update_vrf(rpd, attr->val.string)){ + if(!rad_update_vrf(rpd, attr->val.string, attr->len)){ goto out; } } diff --git a/accel-pppd/radius/radius.c b/accel-pppd/radius/radius.c index fa77a916..89f1e398 100644 --- a/accel-pppd/radius/radius.c +++ b/accel-pppd/radius/radius.c @@ -545,9 +545,16 @@ int rad_proc_attrs(struct rad_req_t *req) } else if (attr->vendor->id == VENDOR_Accel_PPP) { switch (attr->attr->id) { case Accel_VRF_Name: + if (attr->len < 0 || attr->len >= IFNAMSIZ || + memchr(attr->val.string, 0, attr->len)) { + log_ppp_warn("radius: invalid Accel-VRF-Name, rejecting Access-Accept\n"); + return -1; + } if (rpd->ses->vrf_name) _free(rpd->ses->vrf_name); rpd->ses->vrf_name = _malloc(attr->len + 1); + if (!rpd->ses->vrf_name) + return -1; memcpy(rpd->ses->vrf_name, attr->val.string, attr->len); rpd->ses->vrf_name[attr->len] = 0; break; -- cgit v1.2.3