From e0c63e6259bb4a75468e5680fb2d10934c2d035b Mon Sep 17 00:00:00 2001 From: Denys Fedoryshchenko Date: Tue, 1 Sep 2026 08:50:12 +0300 Subject: radius: harden packet integer decoding Serialize integer fields via memcpy, restrict attribute parsing to the RADIUS header's declared packet length, reject undersized packet lengths, and reject truncated attribute headers. --- accel-pppd/radius/packet.c | 59 ++++++++++++++++++++++++++++++++++++---------- 1 file changed, 46 insertions(+), 13 deletions(-) diff --git a/accel-pppd/radius/packet.c b/accel-pppd/radius/packet.c index c7e91349..782ecffe 100644 --- a/accel-pppd/radius/packet.c +++ b/accel-pppd/radius/packet.c @@ -35,6 +35,34 @@ static mempool_t packet_pool; static mempool_t attr_pool; static mempool_t buf_pool; +static uint16_t radius_read_u16(const void *ptr) +{ + uint16_t value; + + memcpy(&value, ptr, sizeof(value)); + return ntohs(value); +} + +static uint32_t radius_read_u32(const void *ptr) +{ + uint32_t value; + + memcpy(&value, ptr, sizeof(value)); + return ntohl(value); +} + +static void radius_write_u16(void *ptr, uint16_t value) +{ + value = htons(value); + memcpy(ptr, &value, sizeof(value)); +} + +static void radius_write_u32(void *ptr, uint32_t value) +{ + value = htonl(value); + memcpy(ptr, &value, sizeof(value)); +} + struct rad_packet_t *rad_packet_alloc(int code) { struct rad_packet_t *pack; @@ -111,20 +139,20 @@ int rad_packet_build(struct rad_packet_t *pack, uint8_t *RA) *ptr = pack->code; ptr++; *ptr = pack->id; ptr++; - *(uint16_t*)ptr = htons(pack->len); ptr+= 2; + radius_write_u16(ptr, pack->len); ptr += 2; memcpy(ptr, RA, 16); ptr+=16; list_for_each_entry(attr, &pack->attrs, entry) { if (attr->vendor) { *ptr = 26; ptr++; *ptr = attr->len + 2 + 6; ptr++; - *(uint32_t *)ptr = htonl(attr->vendor->id); ptr+=4; + radius_write_u32(ptr, attr->vendor->id); ptr += 4; } *ptr = attr->attr->id; ptr++; *ptr = attr->len + 2; ptr++; switch(attr->attr->type) { case ATTR_TYPE_INTEGER: - *(uint32_t*)ptr = htonl(attr->val.integer); + radius_write_u32(ptr, attr->val.integer); break; case ATTR_TYPE_OCTETS: case ATTR_TYPE_STRING: @@ -136,7 +164,7 @@ int rad_packet_build(struct rad_packet_t *pack, uint8_t *RA) memcpy(ptr, &attr->val, attr->len); break; case ATTR_TYPE_DATE: - *(uint32_t*)ptr = htonl(attr->val.date); + radius_write_u32(ptr, attr->val.date); break; case ATTR_TYPE_IPV6PREFIX: ptr[0] = 0; @@ -204,17 +232,22 @@ int rad_packet_recv(int fd, struct rad_packet_t **p, struct sockaddr_in *addr) pack->code = *ptr; ptr++; pack->id = *ptr; ptr++; - pack->len = ntohs(*(uint16_t*)ptr); ptr += 2; + pack->len = radius_read_u16(ptr); ptr += 2; - if (pack->len > n) { + if (pack->len < 20 || pack->len > n) { log_ppp_warn("radius:packet: short packet received %i, expected %i\n", pack->len, n); goto out_err; } ptr += 16; - n -= 20; + n = pack->len - 20; while (n>0) { + if (n < 2) { + log_ppp_warn("radius:packet: truncated attribute header received\n"); + goto out_err; + } + id = *ptr; ptr++; len = *ptr - 2; ptr++; if (len < 0) { @@ -230,7 +263,7 @@ int rad_packet_recv(int fd, struct rad_packet_t **p, struct sockaddr_in *addr) log_ppp_warn("radius:packet: vendor attribute too short (%i)\n", len); goto out_err; } - vendor_id = ntohl(*(uint32_t *)ptr); + vendor_id = radius_read_u32(ptr); vendor = rad_dict_find_vendor_id(vendor_id); if (vendor) { if (len < 4 + vendor->tag + vendor->len) { @@ -240,14 +273,14 @@ int rad_packet_recv(int fd, struct rad_packet_t **p, struct sockaddr_in *addr) ptr += 4; if (vendor->tag == 2) - id = (uint16_t)ntohs(*(uint16_t *)ptr); + id = radius_read_u16(ptr); else id = *ptr; ptr += vendor->tag; if (vendor->len == 2) - len = (uint16_t)ntohs(*(uint16_t *)ptr); + len = radius_read_u16(ptr); else len = *ptr; @@ -300,15 +333,15 @@ int rad_packet_recv(int fd, struct rad_packet_t **p, struct sockaddr_in *addr) break; } if (len == 4) - attr->val.integer = ntohl(*(uint32_t*)ptr); + attr->val.integer = radius_read_u32(ptr); else if (len == 2) - attr->val.integer = ntohs(*(uint16_t*)ptr); + attr->val.integer = radius_read_u16(ptr); else if (len == 1) attr->val.integer = *ptr; break; case ATTR_TYPE_DATE: if (len == 4) - attr->val.integer = ntohl(*(uint32_t*)ptr); + attr->val.integer = radius_read_u32(ptr); else log_ppp_warn("radius:packet: attribute %s has invalid length %i (must be 4)\n", da->name, len); break; -- cgit v1.2.3