From 4654c4a9c083780f5e151ee064e69a357c48d364 Mon Sep 17 00:00:00 2001 From: Denys Fedoryshchenko Date: Tue, 1 Sep 2026 09:32:30 +0300 Subject: 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. --- accel-pppd/ppp/ppp_ccp.c | 9 +++++++-- accel-pppd/ppp/ppp_ipcp.c | 9 +++++++-- accel-pppd/ppp/ppp_ipv6cp.c | 9 +++++++-- accel-pppd/ppp/ppp_lcp.c | 9 +++++++-- 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)); -- cgit v1.2.3