Merge pull request #361 from nuclearcat/fix-protocol-buffer-access

Fix several unsafe or unaligned integer accesses found in protocol parsing paths, including option-gated MPPE, DHCP, PPPoE, RADIUS, IPCP and IPv6CP code.

6 files changed, 168 insertions, 41 deletions

diff --git a/accel-pppd/ppp/ccp_mppe.c b/accel-pppd/ppp/ccp_mppe.c
index cb41c0da..5042c8a0 100644
--- a/accel-pppd/ppp/ccp_mppe.c
+++ b/accel-pppd/ppp/ccp_mppe.c
@@ -10,6 +10,7 @@
 #include "ppp_ccp.h"
 #include "log.h"
 #include "events.h"
+#include "utils.h"
 
 #include "memdebug.h"
 
@@ -106,7 +107,7 @@ static int setup_mppe_key(int fd, int transmit, uint8_t *key)
 	memset(buf, 0, sizeof(buf));
 	buf[0] = CI_MPPE;
 	buf[1] = 6;
-	*(uint32_t*)(buf + 2) = htonl(MPPE_S | MPPE_H);
+	u_write_be32(buf + 2, MPPE_S | MPPE_H);
 	if (key)
 		memcpy(buf + 6, key, 16);
 
diff --git a/accel-pppd/ppp/ipv6cp_opt_intfid.c b/accel-pppd/ppp/ipv6cp_opt_intfid.c
index cb33f024..5de88932 100644
--- a/accel-pppd/ppp/ipv6cp_opt_intfid.c
+++ b/accel-pppd/ppp/ipv6cp_opt_intfid.c
@@ -284,12 +284,14 @@ static void ipaddr_print(void (*print)(const char *fmt,...), struct ipv6cp_optio
 {
 	struct ipaddr_option_t *ipaddr_opt = container_of(opt, typeof(*ipaddr_opt), opt);
 	struct ipv6cp_opt64_t *opt64 = (struct ipv6cp_opt64_t *)ptr;
-	struct in6_addr a;
+	struct in6_addr a = {};
+	uint64_t intf_id;
 
 	if (ptr)
-		*(uint64_t *)(a.s6_addr + 8) = opt64->val;
+		intf_id = opt64->val;
 	else
-		*(uint64_t *)(a.s6_addr + 8) = ipaddr_opt->ppp->ses.ipv6->intf_id;
+		intf_id = ipaddr_opt->ppp->ses.ipv6->intf_id;
+	memcpy(a.s6_addr + 8, &intf_id, sizeof(intf_id));
 
 	print("<addr %x:%x:%x:%x>", ntohs(a.s6_addr16[4]), ntohs(a.s6_addr16[5]), ntohs(a.s6_addr16[6]), ntohs(a.s6_addr16[7]));
 }
@@ -376,4 +378,3 @@ static void init()
 }
 
 DEFINE_INIT(5, init);
-
diff --git a/accel-pppd/ppp/ppp_ccp.c b/accel-pppd/ppp/ppp_ccp.c
index f9e05e89..2082ca3f 100644
--- a/accel-pppd/ppp/ppp_ccp.c
+++ b/accel-pppd/ppp/ppp_ccp.c
@@ -387,10 +387,18 @@ static int ccp_recv_conf_req(struct ppp_ccp_t *ccp, uint8_t *data, int size)
 	ccp->ropt_len = size;
 
 	while (size > 0) {
+		if (size < sizeof(*hdr)) {
+			log_ppp_warn("CCP: ConfReq: truncated option header (%i bytes left)\n", size);
+			return CCP_OPT_FAIL;
+		}
+
 		hdr = (struct ccp_opt_hdr_t *)data;
 
-		if (!hdr->len || hdr->len > size)
-			break;
+		if (hdr->len < sizeof(*hdr) || hdr->len > size) {
+			log_ppp_warn("CCP: ConfReq: invalid length %i of option %i (%i bytes left)\n",
+				     hdr->len, hdr->id, size);
+			return CCP_OPT_FAIL;
+		}
 
 		ropt = _malloc(sizeof(*ropt));
 		memset(ropt, 0, sizeof(*ropt));
@@ -482,10 +490,17 @@ static int ccp_recv_conf_rej(struct ppp_ccp_t *ccp, uint8_t *data, int size)
 	}*/
 
 	while (size > 0) {
+		if (size < sizeof(*hdr)) {
+			res = -1;
+			break;
+		}
+
 		hdr = (struct ccp_opt_hdr_t *)data;
 
-		if (!hdr->len || hdr->len > size)
+		if (hdr->len < sizeof(*hdr) || hdr->len > size) {
+			res = -1;
 			break;
+		}
 
 		list_for_each_entry(lopt, &ccp->options, entry) {
 			if (lopt->id == hdr->id) {
@@ -523,10 +538,17 @@ static int ccp_recv_conf_nak(struct ppp_ccp_t *ccp, uint8_t *data, int size)
 	}*/
 
 	while (size > 0) {
+		if (size < sizeof(*hdr)) {
+			res = -1;
+			break;
+		}
+
 		hdr = (struct ccp_opt_hdr_t *)data;
 
-		if (!hdr->len || hdr->len > size)
+		if (hdr->len < sizeof(*hdr) || hdr->len > size) {
+			res = -1;
 			break;
+		}
 
 		list_for_each_entry(lopt, &ccp->options, entry) {
 			if (lopt->id == hdr->id) {
@@ -566,10 +588,17 @@ static int ccp_recv_conf_ack(struct ppp_ccp_t *ccp, uint8_t *data, int size)
 	}*/
 
 	while (size > 0) {
+		if (size < sizeof(*hdr)) {
+			res = -1;
+			break;
+		}
+
 		hdr = (struct ccp_opt_hdr_t *)data;
 
-		if (!hdr->len || hdr->len > size)
+		if (hdr->len < sizeof(*hdr) || hdr->len > size) {
+			res = -1;
 			break;
+		}
 
 		list_for_each_entry(lopt, &ccp->options, entry) {
 			if (lopt->id == hdr->id) {
@@ -647,7 +676,7 @@ static void ccp_recv(struct ppp_handler_t*h)
 	}
 
 	hdr = (struct ccp_hdr_t *)ccp->ppp->buf;
-	if (ntohs(hdr->len) < PPP_HEADERLEN) {
+	if (ntohs(hdr->len) < PPP_HEADERLEN || ntohs(hdr->len) > ccp->ppp->buf_size - 2) {
 		log_ppp_warn("CCP: short packet received\n");
 		return;
 	}
@@ -695,8 +724,10 @@ static void ccp_recv(struct ppp_handler_t*h)
 				ppp_fsm_recv_conf_ack(&ccp->fsm);
 			break;
 		case CONFNAK:
-			ccp_recv_conf_nak(ccp, (uint8_t*)(hdr + 1), ntohs(hdr->len) - PPP_HDRLEN);
-			ppp_fsm_recv_conf_rej(&ccp->fsm);
+			if (ccp_recv_conf_nak(ccp, (uint8_t*)(hdr + 1), ntohs(hdr->len) - PPP_HDRLEN))
+				ap_session_terminate(&ccp->ppp->ses, TERM_USER_ERROR, 0);
+			else
+				ppp_fsm_recv_conf_rej(&ccp->fsm);
 			break;
 		case CONFREJ:
 			if (ccp_recv_conf_rej(ccp, (uint8_t*)(hdr + 1),ntohs(hdr->len) - PPP_HDRLEN))
diff --git a/accel-pppd/ppp/ppp_ipcp.c b/accel-pppd/ppp/ppp_ipcp.c
index 416fba93..2f75542a 100644
--- a/accel-pppd/ppp/ppp_ipcp.c
+++ b/accel-pppd/ppp/ppp_ipcp.c
@@ -391,10 +391,18 @@ static int ipcp_recv_conf_req(struct ppp_ipcp_t *ipcp, uint8_t *data, int size)
 	ipcp->ropt_len = size;
 
 	while (size > 0) {
+		if (size < sizeof(*hdr)) {
+			log_ppp_warn("IPCP: ConfReq: truncated option header (%i bytes left)\n", size);
+			return IPCP_OPT_FAIL;
+		}
+
 		hdr = (struct ipcp_opt_hdr_t *)data;
 
-		if (!hdr->len || hdr->len > size)
-			break;
+		if (hdr->len < sizeof(*hdr) || hdr->len > size) {
+			log_ppp_warn("IPCP: ConfReq: invalid length %i of option %i (%i bytes left)\n",
+				     hdr->len, hdr->id, size);
+			return IPCP_OPT_FAIL;
+		}
 
 		ropt = _malloc(sizeof(*ropt));
 		memset(ropt, 0, sizeof(*ropt));
@@ -503,10 +511,17 @@ static int ipcp_recv_conf_rej(struct ppp_ipcp_t *ipcp, uint8_t *data, int size)
 	}*/
 
 	while (size > 0) {
+		if (size < sizeof(*hdr)) {
+			res = -1;
+			break;
+		}
+
 		hdr = (struct ipcp_opt_hdr_t *)data;
 
-		if (!hdr->len || hdr->len > size)
+		if (hdr->len < sizeof(*hdr) || hdr->len > size) {
+			res = -1;
 			break;
+		}
 
 		list_for_each_entry(lopt, &ipcp->options, entry) {
 			if (lopt->id == hdr->id) {
@@ -544,10 +559,17 @@ static int ipcp_recv_conf_nak(struct ppp_ipcp_t *ipcp, uint8_t *data, int size)
 	}*/
 
 	while (size > 0) {
+		if (size < sizeof(*hdr)) {
+			res = -1;
+			break;
+		}
+
 		hdr = (struct ipcp_opt_hdr_t *)data;
 
-		if (!hdr->len || hdr->len > size)
+		if (hdr->len < sizeof(*hdr) || hdr->len > size) {
+			res = -1;
 			break;
+		}
 
 		list_for_each_entry(lopt, &ipcp->options, entry) {
 			if (lopt->id == hdr->id) {
@@ -587,10 +609,17 @@ static int ipcp_recv_conf_ack(struct ppp_ipcp_t *ipcp, uint8_t *data, int size)
 	}*/
 
 	while (size > 0) {
+		if (size < sizeof(*hdr)) {
+			res = -1;
+			break;
+		}
+
 		hdr = (struct ipcp_opt_hdr_t *)data;
 
-		if (!hdr->len || hdr->len > size)
+		if (hdr->len < sizeof(*hdr) || hdr->len > size) {
+			res = -1;
 			break;
+		}
 
 		list_for_each_entry(lopt, &ipcp->options, entry) {
 			if (lopt->id == hdr->id) {
@@ -671,7 +700,7 @@ static void ipcp_recv(struct ppp_handler_t*h)
 	}
 
 	hdr = (struct ipcp_hdr_t *)ipcp->ppp->buf;
-	if (ntohs(hdr->len) < PPP_HEADERLEN) {
+	if (ntohs(hdr->len) < PPP_HEADERLEN || ntohs(hdr->len) > ipcp->ppp->buf_size - 2) {
 		log_ppp_warn("IPCP: short packet received\n");
 		return;
 	}
@@ -725,8 +754,10 @@ static void ipcp_recv(struct ppp_handler_t*h)
 				ppp_fsm_recv_conf_ack(&ipcp->fsm);
 			break;
 		case CONFNAK:
-			ipcp_recv_conf_nak(ipcp,(uint8_t*)(hdr + 1), ntohs(hdr->len) - PPP_HDRLEN);
-			ppp_fsm_recv_conf_rej(&ipcp->fsm);
+			if (ipcp_recv_conf_nak(ipcp,(uint8_t*)(hdr + 1), ntohs(hdr->len) - PPP_HDRLEN))
+				ap_session_terminate(&ipcp->ppp->ses, TERM_USER_ERROR, 0);
+			else
+				ppp_fsm_recv_conf_rej(&ipcp->fsm);
 			break;
 		case CONFREJ:
 			if (ipcp_recv_conf_rej(ipcp, (uint8_t*)(hdr + 1), ntohs(hdr->len) - PPP_HDRLEN))
diff --git a/accel-pppd/ppp/ppp_ipv6cp.c b/accel-pppd/ppp/ppp_ipv6cp.c
index 7f278daa..370f1d55 100644
--- a/accel-pppd/ppp/ppp_ipv6cp.c
+++ b/accel-pppd/ppp/ppp_ipv6cp.c
@@ -395,10 +395,18 @@ static int ipv6cp_recv_conf_req(struct ppp_ipv6cp_t *ipv6cp, uint8_t *data, int
 	ipv6cp->ropt_len = size;
 
 	while (size > 0) {
+		if (size < sizeof(*hdr)) {
+			log_ppp_warn("IPV6CP: ConfReq: truncated option header (%i bytes left)\n", size);
+			return IPV6CP_OPT_FAIL;
+		}
+
 		hdr = (struct ipv6cp_opt_hdr_t *)data;
 
-		if (!hdr->len || hdr->len > size)
-			break;
+		if (hdr->len < sizeof(*hdr) || hdr->len > size) {
+			log_ppp_warn("IPV6CP: ConfReq: invalid length %i of option %i (%i bytes left)\n",
+				     hdr->len, hdr->id, size);
+			return IPV6CP_OPT_FAIL;
+		}
 
 		ropt = _malloc(sizeof(*ropt));
 		memset(ropt, 0, sizeof(*ropt));
@@ -507,10 +515,17 @@ static int ipv6cp_recv_conf_rej(struct ppp_ipv6cp_t *ipv6cp, uint8_t *data, int
 	}*/
 
 	while (size > 0) {
+		if (size < sizeof(*hdr)) {
+			res = -1;
+			break;
+		}
+
 		hdr = (struct ipv6cp_opt_hdr_t *)data;
 
-		if (!hdr->len || hdr->len > size)
+		if (hdr->len < sizeof(*hdr) || hdr->len > size) {
+			res = -1;
 			break;
+		}
 
 		list_for_each_entry(lopt, &ipv6cp->options, entry) {
 			if (lopt->id == hdr->id) {
@@ -548,10 +563,17 @@ static int ipv6cp_recv_conf_nak(struct ppp_ipv6cp_t *ipv6cp, uint8_t *data, int
 	}*/
 
 	while (size > 0) {
+		if (size < sizeof(*hdr)) {
+			res = -1;
+			break;
+		}
+
 		hdr = (struct ipv6cp_opt_hdr_t *)data;
 
-		if (!hdr->len || hdr->len > size)
+		if (hdr->len < sizeof(*hdr) || hdr->len > size) {
+			res = -1;
 			break;
+		}
 
 		list_for_each_entry(lopt, &ipv6cp->options, entry) {
 			if (lopt->id == hdr->id) {
@@ -591,10 +613,17 @@ static int ipv6cp_recv_conf_ack(struct ppp_ipv6cp_t *ipv6cp, uint8_t *data, int
 	}*/
 
 	while (size > 0) {
+		if (size < sizeof(*hdr)) {
+			res = -1;
+			break;
+		}
+
 		hdr = (struct ipv6cp_opt_hdr_t *)data;
 
-		if (!hdr->len || hdr->len > size)
+		if (hdr->len < sizeof(*hdr) || hdr->len > size) {
+			res = -1;
 			break;
+		}
 
 		list_for_each_entry(lopt, &ipv6cp->options, entry) {
 			if (lopt->id == hdr->id) {
@@ -675,7 +704,7 @@ static void ipv6cp_recv(struct ppp_handler_t*h)
 	}
 
 	hdr = (struct ipv6cp_hdr_t *)ipv6cp->ppp->buf;
-	if (ntohs(hdr->len) < PPP_HEADERLEN) {
+	if (ntohs(hdr->len) < PPP_HEADERLEN || ntohs(hdr->len) > ipv6cp->ppp->buf_size - 2) {
 		log_ppp_warn("IPV6CP: short packet received\n");
 		return;
 	}
@@ -729,8 +758,10 @@ static void ipv6cp_recv(struct ppp_handler_t*h)
 				ppp_fsm_recv_conf_ack(&ipv6cp->fsm);
 			break;
 		case CONFNAK:
-			ipv6cp_recv_conf_nak(ipv6cp,(uint8_t*)(hdr + 1), ntohs(hdr->len) - PPP_HDRLEN);
-			ppp_fsm_recv_conf_rej(&ipv6cp->fsm);
+			if (ipv6cp_recv_conf_nak(ipv6cp,(uint8_t*)(hdr + 1), ntohs(hdr->len) - PPP_HDRLEN))
+				ap_session_terminate(&ipv6cp->ppp->ses, TERM_USER_ERROR, 0);
+			else
+				ppp_fsm_recv_conf_rej(&ipv6cp->fsm);
 			break;
 		case CONFREJ:
 			if (ipv6cp_recv_conf_rej(ipv6cp, (uint8_t*)(hdr + 1), ntohs(hdr->len) - PPP_HDRLEN))
diff --git a/accel-pppd/ppp/ppp_lcp.c b/accel-pppd/ppp/ppp_lcp.c
index ed085b3b..2424ca94 100644
--- a/accel-pppd/ppp/ppp_lcp.c
+++ b/accel-pppd/ppp/ppp_lcp.c
@@ -370,10 +370,18 @@ static int lcp_recv_conf_req(struct ppp_lcp_t *lcp, uint8_t *data, int size)
 	lcp->ropt_len = size;
 
 	while (size > 0) {
+		if (size < sizeof(*hdr)) {
+			log_ppp_warn("LCP: ConfReq: truncated option header (%i bytes left)\n", size);
+			return LCP_OPT_FAIL;
+		}
+
 		hdr = (struct lcp_opt_hdr_t *)data;
 
-		if (!hdr->len || hdr->len > size)
-			break;
+		if (hdr->len < sizeof(*hdr) || hdr->len > size) {
+			log_ppp_warn("LCP: ConfReq: invalid length %i of option %i (%i bytes left)\n",
+				     hdr->len, hdr->id, size);
+			return LCP_OPT_FAIL;
+		}
 
 		ropt = _malloc(sizeof(*ropt));
 		memset(ropt, 0, sizeof(*ropt));
@@ -462,10 +470,17 @@ static int lcp_recv_conf_rej(struct ppp_lcp_t *lcp, uint8_t *data, int size)
 	}
 
 	while (size > 0) {
+		if (size < sizeof(*hdr)) {
+			res = -1;
+			break;
+		}
+
 		hdr = (struct lcp_opt_hdr_t *)data;
 
-		if (!hdr->len || hdr->len > size)
+		if (hdr->len < sizeof(*hdr) || hdr->len > size) {
+			res = -1;
 			break;
+		}
 
 		list_for_each_entry(lopt, &lcp->options, entry) {
 			if (lopt->id == hdr->id) {
@@ -507,10 +522,17 @@ static int lcp_recv_conf_nak(struct ppp_lcp_t *lcp, uint8_t *data, int size)
 	}
 
 	while (size > 0) {
+		if (size < sizeof(*hdr)) {
+			res = -1;
+			break;
+		}
+
 		hdr = (struct lcp_opt_hdr_t *)data;
 
-		if (!hdr->len || hdr->len > size)
+		if (hdr->len < sizeof(*hdr) || hdr->len > size) {
+			res = -1;
 			break;
+		}
 
 		list_for_each_entry(lopt,&lcp->options,entry) {
 			if (lopt->id == hdr->id) {
@@ -550,10 +572,17 @@ static int lcp_recv_conf_ack(struct ppp_lcp_t *lcp, uint8_t *data, int size)
 	}
 
 	while (size > 0) {
+		if (size < sizeof(*hdr)) {
+			res = -1;
+			break;
+		}
+
 		hdr = (struct lcp_opt_hdr_t *)data;
 
-		if (!hdr->len || hdr->len > size)
+		if (hdr->len < sizeof(*hdr) || hdr->len > size) {
+			res = -1;
 			break;
+		}
 
 		list_for_each_entry(lopt, &lcp->options, entry) {
 			if (lopt->id == hdr->id) {
@@ -587,7 +616,7 @@ static void lcp_recv_echo_repl(struct ppp_lcp_t *lcp, uint8_t *data, int size)
 		if (conf_ppp_verbose)
 			log_ppp_debug("recv [LCP EchoRep id=%x]\n", lcp->fsm.recv_id);
 	} else {
-		magic = ntohl(*(uint32_t *)data);
+		magic = u_read_be32(data);
 
 		if (conf_ppp_verbose)
 			log_ppp_debug("recv [LCP EchoRep id=%x <magic %08x>]\n", lcp->fsm.recv_id, magic);
@@ -746,7 +775,7 @@ static void lcp_recv(struct ppp_handler_t*h)
 	hdr = (struct lcp_hdr_t *)lcp->ppp->buf;
 	len = ntohs(hdr->len);
 	buf_len = lcp->ppp->buf_size;
-	if (len < PPP_HEADERLEN) {
+	if (len < PPP_HEADERLEN || len > lcp->ppp->buf_size - 2) {
 		log_ppp_warn("LCP: short packet received\n");
 		return;
 	}
@@ -802,7 +831,10 @@ static void lcp_recv(struct ppp_handler_t*h)
 			}
 			break;
 		case CONFNAK:
-			lcp_recv_conf_nak(lcp, (uint8_t*)(hdr + 1), ntohs(hdr->len) - PPP_HDRLEN);
+			if (lcp_recv_conf_nak(lcp, (uint8_t*)(hdr + 1), ntohs(hdr->len) - PPP_HDRLEN)) {
+				ap_session_terminate(&lcp->ppp->ses, TERM_USER_ERROR, 0);
+				break;
+			}
 			if (lcp->fsm.recv_id != lcp->fsm.id)
 				break;
 			ppp_fsm_recv_conf_rej(&lcp->fsm);
@@ -838,7 +870,7 @@ static void lcp_recv(struct ppp_handler_t*h)
 				break;
 			}
 			if (conf_ppp_verbose)
-				log_ppp_debug("recv [LCP EchoReq id=%x <magic %08x>]\n", hdr->id, ntohl(*(uint32_t*)(hdr + 1)));
+				log_ppp_debug("recv [LCP EchoReq id=%x <magic %08x>]\n", hdr->id, u_read_be32(hdr + 1));
 			send_echo_reply(lcp);
 			break;
 		case ECHOREP:
@@ -854,11 +886,11 @@ static void lcp_recv(struct ppp_handler_t*h)
 					log_ppp_warn("LCP: short ProtoRej received\n");
 					break;
 				}
-				log_ppp_info2("recv [LCP ProtoRej id=%x <%04x>]\n", hdr->id, ntohs(*(uint16_t*)(hdr + 1)));
+				log_ppp_info2("recv [LCP ProtoRej id=%x <%04x>]\n", hdr->id, u_read_be16(hdr + 1));
 			}
 			if (len < PPP_HDRLEN + 2 || buf_len < (int)(sizeof(*hdr) + 2))
 				break;
-			ppp_recv_proto_rej(lcp->ppp, ntohs(*(uint16_t *)(hdr + 1)));
+			ppp_recv_proto_rej(lcp->ppp, u_read_be16(hdr + 1));
 			break;
 		case DISCARDREQ:
 			if (conf_ppp_verbose) {
@@ -866,7 +898,7 @@ static void lcp_recv(struct ppp_handler_t*h)
 					log_ppp_warn("LCP: short DiscardReq received\n");
 					break;
 				}
-				log_ppp_info2("recv [LCP DiscardReq id=%x <magic %08x>]\n", hdr->id, ntohl(*(uint32_t*)(hdr + 1)));
+				log_ppp_info2("recv [LCP DiscardReq id=%x <magic %08x>]\n", hdr->id, u_read_be32(hdr + 1));
 			}
 			break;
 		case IDENT: