ppp: say why a malformed ConfReq drops the session

The option walkers of lcp_recv_conf_req() and its IPCP, IPV6CP and CCP
counterparts bail out with *_OPT_FAIL as soon as an option header does not
fit in the remaining bytes or carries an impossible length, and the caller
turns that into ap_session_terminate(TERM_USER_ERROR).

The bail out happens before the "recv [LCP ConfReq id=..." line is emitted,
so the session simply disappeared with nothing in the log to point at the
peer. The ConfAck, ConfNak and ConfRej walkers break out of their loop
instead and at least close the line they had already started.

Log the offending length, option id and the number of bytes left before
returning, so a peer sending malformed options can be told apart from the
other reasons a session ends with TERM_USER_ERROR.

4 files changed, 28 insertions, 8 deletions

diff --git a/accel-pppd/ppp/ppp_ccp.c b/accel-pppd/ppp/ppp_ccp.c
index 4dda10f7..2082ca3f 100644
--- a/accel-pppd/ppp/ppp_ccp.c
+++ b/accel-pppd/ppp/ppp_ccp.c
@@ -387,13 +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))
+		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 < sizeof(*hdr) || hdr->len > size)
+		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));
diff --git a/accel-pppd/ppp/ppp_ipcp.c b/accel-pppd/ppp/ppp_ipcp.c
index a919883a..2f75542a 100644
--- a/accel-pppd/ppp/ppp_ipcp.c
+++ b/accel-pppd/ppp/ppp_ipcp.c
@@ -391,13 +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))
+		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 < sizeof(*hdr) || hdr->len > size)
+		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));
diff --git a/accel-pppd/ppp/ppp_ipv6cp.c b/accel-pppd/ppp/ppp_ipv6cp.c
index 5f08e46c..370f1d55 100644
--- a/accel-pppd/ppp/ppp_ipv6cp.c
+++ b/accel-pppd/ppp/ppp_ipv6cp.c
@@ -395,13 +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))
+		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 < sizeof(*hdr) || hdr->len > size)
+		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));
diff --git a/accel-pppd/ppp/ppp_lcp.c b/accel-pppd/ppp/ppp_lcp.c
index b44674f4..2424ca94 100644
--- a/accel-pppd/ppp/ppp_lcp.c
+++ b/accel-pppd/ppp/ppp_lcp.c
@@ -370,13 +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))
+		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 < sizeof(*hdr) || hdr->len > size)
+		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));