From 5ca46e28fd1f0e055c40546b1c698fc970b6e114 Mon Sep 17 00:00:00 2001 From: Denys Fedoryshchenko Date: Sun, 9 Aug 2026 07:05:38 +0300 Subject: pptp: fix truncated control messages on partial write post_msg() copies the unsent tail of a message into conn->out_buf and enables the write handler, but never sets conn->out_size. pptp_write() then computes out_size - out_pos as 0, writes nothing, sees out_pos == out_size, disables itself and returns, so the buffered remainder is silently dropped. post_msg() still returns 0, so the caller believes the message was sent. The usual trigger is a peer that stops reading: once the send buffer fills, write() returns EAGAIN, n is set to 0 and the whole message is buffered and then discarded, losing replies such as Start-Ctrl-Conn-Reply, Outgoing-Call-Reply and Call-Disconnect-Notify. Record the remaining length so pptp_write() can flush it. out_pos is already 0 here: post_msg() returns early unless out_size is 0, which holds only before the first send or after pptp_write() has drained the buffer and reset both fields. --- accel-pppd/ctrl/pptp/pptp.c | 1 + 1 file changed, 1 insertion(+) (limited to 'accel-pppd') diff --git a/accel-pppd/ctrl/pptp/pptp.c b/accel-pppd/ctrl/pptp/pptp.c index f8e498d6..232c9fb5 100644 --- a/accel-pppd/ctrl/pptp/pptp.c +++ b/accel-pppd/ctrl/pptp/pptp.c @@ -202,6 +202,7 @@ again: if ( nout_buf, (uint8_t *)buf + n, size - n); + conn->out_size = size - n; triton_md_enable_handler(&conn->hnd, MD_MODE_WRITE); } -- cgit v1.2.3 From 2ad648c3cdde7c5b439abde9566c6dedce966c9e Mon Sep 17 00:00:00 2001 From: Denys Fedoryshchenko Date: Sun, 9 Aug 2026 07:15:01 +0300 Subject: pptp: reject control messages shorter than the header PPTP_CTRL_SIZE() evaluates to 0 for unrecognised control types, so a message declaring length 0 with such a type passed the length check, reached process_packet() and was logged as unknown, after which in_size -= 0 consumed nothing. The stale header stayed at the head of the buffer and every later byte queued behind it, so the connection could never make progress: it stalled until in_size reached PPTP_CTRL_SIZE_MAX, at which point read() was called with a zero-length buffer, returned 0 and was misreported as "disconnect by peer". Require the declared length to cover the header, alongside the existing upper bound. --- accel-pppd/ctrl/pptp/pptp.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'accel-pppd') diff --git a/accel-pppd/ctrl/pptp/pptp.c b/accel-pppd/ctrl/pptp/pptp.c index 232c9fb5..b323dba6 100644 --- a/accel-pppd/ctrl/pptp/pptp.c +++ b/accel-pppd/ctrl/pptp/pptp.c @@ -567,6 +567,10 @@ static int pptp_read(struct triton_md_handler_t *h) log_ppp_error("pptp: invalid magic\n"); goto drop; } + if (ntohs(hdr->length) < sizeof(*hdr)) { + log_ppp_error("pptp: message is too short\n"); + goto drop; + } if (ntohs(hdr->length) >= PPTP_CTRL_SIZE_MAX) { log_ppp_error("pptp: message is too long\n"); goto drop; -- cgit v1.2.3 From f49ed06b26b2b3ea3a97589be2e51ceb2a084243 Mon Sep 17 00:00:00 2001 From: Denys Fedoryshchenko Date: Sun, 9 Aug 2026 07:13:45 +0300 Subject: pptp: close call socket when Outgoing-Call-Reply cannot be sent The PPPoX socket created for the call is only handed to conn->ppp.fd after the reply has been posted, so returning early on a post_msg() failure leaks the descriptor: disconnect() knows nothing about it and establish_ppp() has not run yet. The establish_ppp() failure path just below already closes it. --- accel-pppd/ctrl/pptp/pptp.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'accel-pppd') diff --git a/accel-pppd/ctrl/pptp/pptp.c b/accel-pppd/ctrl/pptp/pptp.c index b323dba6..20f8495d 100644 --- a/accel-pppd/ctrl/pptp/pptp.c +++ b/accel-pppd/ctrl/pptp/pptp.c @@ -393,8 +393,10 @@ static int pptp_out_call_rqst(struct pptp_conn_t *conn) return -1; } - if (send_pptp_out_call_rply(conn, msg, src_addr.sa_addr.pptp.call_id, PPTP_CALL_RES_OK, 0)) + if (send_pptp_out_call_rply(conn, msg, src_addr.sa_addr.pptp.call_id, PPTP_CALL_RES_OK, 0)) { + close(pptp_sock); return -1; + } conn->call_id = src_addr.sa_addr.pptp.call_id; conn->peer_call_id = msg->call_id; -- cgit v1.2.3 From a62e3abe2aae259f90289b431e086490020d49ed Mon Sep 17 00:00:00 2001 From: Denys Fedoryshchenko Date: Sun, 9 Aug 2026 07:14:41 +0300 Subject: pptp: check getsockname()/getpeername() results Both calls were issued on an uninitialised struct sockaddr_in and their results ignored, so a failure would build the tunnel endpoints, and the call socket's local call id, out of stack garbage. Fail the call instead. In pptp_connect() the local address was fetched into the same variable that held the peer address obtained from accept(), so a failure there would silently set called-station-id to the calling station. Read it into its own variable before the connection is set up, and re-arm the address length before each accept() rather than leaving it at whatever the previous iteration wrote. --- accel-pppd/ctrl/pptp/pptp.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) (limited to 'accel-pppd') diff --git a/accel-pppd/ctrl/pptp/pptp.c b/accel-pppd/ctrl/pptp/pptp.c index 20f8495d..cc4d222a 100644 --- a/accel-pppd/ctrl/pptp/pptp.c +++ b/accel-pppd/ctrl/pptp/pptp.c @@ -360,7 +360,10 @@ static int pptp_out_call_rqst(struct pptp_conn_t *conn) src_addr.sa_protocol = PX_PROTO_PPTP; src_addr.sa_addr.pptp.call_id = 0; addrlen = sizeof(addr); - getsockname(conn->hnd.fd, (struct sockaddr*)&addr, &addrlen); + if (getsockname(conn->hnd.fd, (struct sockaddr*)&addr, &addrlen)) { + log_ppp_error("pptp: getsockname: %s\n", strerror(errno)); + return -1; + } src_addr.sa_addr.pptp.sin_addr = addr.sin_addr; memset(&dst_addr, 0, sizeof(dst_addr)); @@ -368,7 +371,10 @@ static int pptp_out_call_rqst(struct pptp_conn_t *conn) dst_addr.sa_protocol = PX_PROTO_PPTP; dst_addr.sa_addr.pptp.call_id = htons(msg->call_id); addrlen = sizeof(addr); - getpeername(conn->hnd.fd, (struct sockaddr*)&addr, &addrlen); + if (getpeername(conn->hnd.fd, (struct sockaddr*)&addr, &addrlen)) { + log_ppp_error("pptp: getpeername: %s\n", strerror(errno)); + return -1; + } dst_addr.sa_addr.pptp.sin_addr = addr.sin_addr; pptp_sock = socket(AF_PPPOX, SOCK_STREAM, PX_PROTO_PPTP); @@ -385,7 +391,11 @@ static int pptp_out_call_rqst(struct pptp_conn_t *conn) return -1; } addrlen = sizeof(src_addr); - getsockname(pptp_sock, (struct sockaddr*)&src_addr, &addrlen); + if (getsockname(pptp_sock, (struct sockaddr*)&src_addr, &addrlen)) { + log_ppp_error("pptp: getsockname: %s\n", strerror(errno)); + close(pptp_sock); + return -1; + } if (connect(pptp_sock, (struct sockaddr*)&dst_addr, sizeof(dst_addr))) { log_ppp_error("failed to connect PPTP socket (%s)\n", strerror(errno)); @@ -687,12 +697,13 @@ static void ppp_finished(struct ap_session *ses) static int pptp_connect(struct triton_md_handler_t *h) { - struct sockaddr_in addr; - socklen_t size = sizeof(addr); + struct sockaddr_in addr, laddr; + socklen_t size; int sock; struct pptp_conn_t *conn; while(1) { + size = sizeof(addr); sock = accept(h->fd, (struct sockaddr *)&addr, &size); if (sock < 0) { if (errno == EAGAIN) @@ -729,6 +740,13 @@ static int pptp_connect(struct triton_md_handler_t *h) continue; } + size = sizeof(laddr); + if (getsockname(sock, (struct sockaddr *)&laddr, &size)) { + log_error("pptp: getsockname: %s, closing connection...\n", strerror(errno)); + close(sock); + continue; + } + if (fcntl(sock, F_SETFL, O_NONBLOCK)) { log_error("pptp: failed to set nonblocking mode: %s, closing connection...\n", strerror(errno)); close(sock); @@ -761,8 +779,7 @@ static int pptp_connect(struct triton_md_handler_t *h) conn->ctrl.calling_station_id = _malloc(17); conn->ctrl.called_station_id = _malloc(17); u_inet_ntoa(addr.sin_addr.s_addr, conn->ctrl.calling_station_id); - getsockname(sock, (struct sockaddr*)&addr, &size); - u_inet_ntoa(addr.sin_addr.s_addr, conn->ctrl.called_station_id); + u_inet_ntoa(laddr.sin_addr.s_addr, conn->ctrl.called_station_id); ppp_init(&conn->ppp); conn->ppp.ses.ctrl = &conn->ctrl; -- cgit v1.2.3 From d76b2f6c6b8f2c510fae5beb24e261901a221d3c Mon Sep 17 00:00:00 2001 From: Denys Fedoryshchenko Date: Sun, 9 Aug 2026 07:07:39 +0300 Subject: pptp: fix byte order of peer call id in Call-Disconnect-Notify conn->peer_call_id is assigned msg->call_id straight from the wire, so it holds a network order value, but send_pptp_call_disconnect_notify() then applies htons() to it. On little-endian hosts the field is swapped twice and a peer call id of 0x1234 is sent as 0x3412, so the peer cannot match the notify to its call. Big-endian hosts are unaffected, as both swaps are no-ops there. Store the call id in host order, which is what the htons() at the point of use expects. Nothing else reads the field. --- accel-pppd/ctrl/pptp/pptp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'accel-pppd') diff --git a/accel-pppd/ctrl/pptp/pptp.c b/accel-pppd/ctrl/pptp/pptp.c index cc4d222a..b55260ba 100644 --- a/accel-pppd/ctrl/pptp/pptp.c +++ b/accel-pppd/ctrl/pptp/pptp.c @@ -409,7 +409,7 @@ static int pptp_out_call_rqst(struct pptp_conn_t *conn) } conn->call_id = src_addr.sa_addr.pptp.call_id; - conn->peer_call_id = msg->call_id; + conn->peer_call_id = ntohs(msg->call_id); conn->ppp.fd = pptp_sock; conn->ppp.ses.chan_name = _strdup(inet_ntoa(dst_addr.sa_addr.pptp.sin_addr)); -- cgit v1.2.3 From a9f666ea84927427a27261b81aa77435d4296d28 Mon Sep 17 00:00:00 2001 From: Denys Fedoryshchenko Date: Sun, 9 Aug 2026 07:15:43 +0300 Subject: pptp: make echo-failure=0 disable the check explicitly load_config() accepts echo-failure=0, but the test for it was "++echo_sent == conf_echo_failure", which can never match once the counter has been incremented, so a zero left dead peers undetected without saying so anywhere. Test the option first and compare with >=, which keeps the behaviour for every configured value and makes the disabled case readable, and document it in accel-ppp.conf.5. --- accel-pppd/accel-ppp.conf.5 | 2 +- accel-pppd/ctrl/pptp/pptp.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) (limited to 'accel-pppd') diff --git a/accel-pppd/accel-ppp.conf.5 b/accel-pppd/accel-ppp.conf.5 index bcddfed4..de900edd 100644 --- a/accel-pppd/accel-ppp.conf.5 +++ b/accel-pppd/accel-ppp.conf.5 @@ -709,7 +709,7 @@ If this option is given and greater than zero then pptp module will send echo-re seconds. .TP .BI "echo-failure=" n -Specifies maximum number of echo-requests may be sent without valid echo-reply, if exceeds connection will be terminated. +Specifies maximum number of echo-requests may be sent without valid echo-reply, if exceeds connection will be terminated. Zero disables the check: echo-requests are still sent, but missing replies never terminate the connection (default 3). .TP .BI "timeout=" n Timeout waiting reply from client in seconds (default 5). diff --git a/accel-pppd/ctrl/pptp/pptp.c b/accel-pppd/ctrl/pptp/pptp.c index b55260ba..6b00666e 100644 --- a/accel-pppd/ctrl/pptp/pptp.c +++ b/accel-pppd/ctrl/pptp/pptp.c @@ -508,7 +508,7 @@ static void pptp_send_echo(struct triton_timer_t *t) .header = PPTP_HEADER_CTRL(PPTP_ECHO_RQST), }; - if (++conn->echo_sent == conf_echo_failure) { + if (conf_echo_failure && ++conn->echo_sent >= conf_echo_failure) { log_ppp_warn("pptp: no echo reply\n"); disconnect(conn); return; -- cgit v1.2.3 From 0d7cd3a4ae6beed4eab485052a5e27b0772f15c6 Mon Sep 17 00:00:00 2001 From: Denys Fedoryshchenko Date: Sun, 9 Aug 2026 07:16:14 +0300 Subject: pptp: pass a proper value to SO_REUSEADDR The option value was the address of the listening descriptor rather than a boolean, so the effect depended on the descriptor number: it enabled SO_REUSEADDR only because that number happened to be non-zero, and would disable it if the daemon were ever started with the lower descriptors closed. Use a dedicated flag, as cli/telnet.c does. --- accel-pppd/ctrl/pptp/pptp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'accel-pppd') diff --git a/accel-pppd/ctrl/pptp/pptp.c b/accel-pppd/ctrl/pptp/pptp.c index 6b00666e..ec054e67 100644 --- a/accel-pppd/ctrl/pptp/pptp.c +++ b/accel-pppd/ctrl/pptp/pptp.c @@ -895,7 +895,7 @@ static void pptp_init(void) { struct sockaddr_in addr; char *opt; - int fd; + int fd, f = 1; fd = socket(AF_PPPOX, SOCK_STREAM, PX_PROTO_PPTP); if (fd >= 0) @@ -925,7 +925,7 @@ static void pptp_init(void) else addr.sin_port = htons(PPTP_PORT); - setsockopt(serv.hnd.fd, SOL_SOCKET, SO_REUSEADDR, &serv.hnd.fd, 4); + setsockopt(serv.hnd.fd, SOL_SOCKET, SO_REUSEADDR, &f, sizeof(f)); if (bind (serv.hnd.fd, (struct sockaddr *) &addr, sizeof (addr)) < 0) { log_emerg("pptp: failed to bind socket: %s\n", strerror(errno)); close(serv.hnd.fd); -- cgit v1.2.3 From 9d47f220a09eb30750f85973dc2f0d4157eb78be Mon Sep 17 00:00:00 2001 From: Denys Fedoryshchenko Date: Sun, 9 Aug 2026 07:16:38 +0300 Subject: pptp: reject a malformed bind address An unparsable bind= value went through inet_addr() unchecked and became 255.255.255.255, so the only symptom was bind() failing with "Cannot assign requested address", which does not point at the configuration. Parse with inet_aton() and name the offending value instead. Also zero the address before filling it in, so the padding passed to bind() is not stack garbage. --- accel-pppd/ctrl/pptp/pptp.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'accel-pppd') diff --git a/accel-pppd/ctrl/pptp/pptp.c b/accel-pppd/ctrl/pptp/pptp.c index ec054e67..7cfccce1 100644 --- a/accel-pppd/ctrl/pptp/pptp.c +++ b/accel-pppd/ctrl/pptp/pptp.c @@ -911,12 +911,17 @@ static void pptp_init(void) fcntl(serv.hnd.fd, F_SETFD, fcntl(serv.hnd.fd, F_GETFD) | FD_CLOEXEC); + memset(&addr, 0, sizeof(addr)); addr.sin_family = AF_INET; opt = conf_get_opt("pptp", "bind"); - if (opt) - addr.sin_addr.s_addr = inet_addr(opt); - else + if (opt) { + if (!inet_aton(opt, &addr.sin_addr)) { + log_emerg("pptp: failed to parse bind address '%s'\n", opt); + close(serv.hnd.fd); + return; + } + } else addr.sin_addr.s_addr = htonl(INADDR_ANY); opt = conf_get_opt("pptp", "port"); -- cgit v1.2.3