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.

1 files 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;