From c32518d76569d833b0470633426ebd1f1cf029f4 Mon Sep 17 00:00:00 2001 From: Denys Fedoryshchenko Date: Tue, 11 Aug 2026 21:29:36 +0300 Subject: pppoe: account for the tag header in the add_tag2 bound add_tag2() checked that the tag payload fits into the packet buffer, but the memcpy() copied the 4 byte tag header as well, so the check was short by sizeof(struct pppoe_tag) - 1 bytes. All callers build the packet in a ETHER_MAX_LEN stack buffer and pass tags taken from the received discovery packet. A PADI carrying a Host-Uniq tag of 1454..1456 bytes therefore made pppoe_send_PADO() write up to 3 bytes of peer supplied data past the end of the buffer. The PADS, PADT and error paths are affected in the same way. Include the tag header in the bound, and drop the tag_len < 0 test which can never be true since ntohs() returns an unsigned value. --- accel-pppd/ctrl/pppoe/pppoe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'accel-pppd/ctrl') diff --git a/accel-pppd/ctrl/pppoe/pppoe.c b/accel-pppd/ctrl/pppoe/pppoe.c index 0cc23180..d5896a59 100644 --- a/accel-pppd/ctrl/pppoe/pppoe.c +++ b/accel-pppd/ctrl/pppoe/pppoe.c @@ -792,7 +792,7 @@ static int add_tag2(uint8_t *pack, size_t pack_size, const struct pppoe_tag *t) { struct pppoe_hdr *hdr = (struct pppoe_hdr *)(pack + ETH_HLEN); struct pppoe_tag *tag = (struct pppoe_tag *)(pack + ETH_HLEN + sizeof(*hdr) + ntohs(hdr->length)); - if (pack_size <= ETH_HLEN + sizeof(*hdr) + ntohs(hdr->length) + ntohs(t->tag_len) || ntohs(t->tag_len) < 0) + if (pack_size <= ETH_HLEN + sizeof(*hdr) + ntohs(hdr->length) + sizeof(*t) + ntohs(t->tag_len)) return -1; memcpy(tag, t, sizeof(*t) + ntohs(t->tag_len)); -- cgit v1.2.3 From 86e71f2aa48d62e1c63253f0986d5f643410a263 Mon Sep 17 00:00:00 2001 From: Denys Fedoryshchenko Date: Tue, 11 Aug 2026 21:29:49 +0300 Subject: pppoe: check for a truncated tag header in PADI The PADI tag loop read tag_len before checking that the tag header itself fits into the declared payload length, so a PADI ending with a partial tag made it read up to 2 bytes past the receive buffer. print_packet() and the PADR loop already have this check, add the missing one. --- accel-pppd/ctrl/pppoe/pppoe.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'accel-pppd/ctrl') diff --git a/accel-pppd/ctrl/pppoe/pppoe.c b/accel-pppd/ctrl/pppoe/pppoe.c index d5896a59..72faab4e 100644 --- a/accel-pppd/ctrl/pppoe/pppoe.c +++ b/accel-pppd/ctrl/pppoe/pppoe.c @@ -1043,6 +1043,8 @@ static void pppoe_recv_PADI(struct pppoe_serv_t *serv, uint8_t *pack, int size) len = ntohs(hdr->length); for (n = 0; n < len; n += sizeof(*tag) + ntohs(tag->tag_len)) { tag = (struct pppoe_tag *)(pack + ETH_HLEN + sizeof(*hdr) + n); + if (n + sizeof(*tag) > len) + return; if (n + sizeof(*tag) + ntohs(tag->tag_len) > len) return; switch (ntohs(tag->tag_type)) { -- cgit v1.2.3 From 1c69485e1ebd17bf3cc6e8fc4728f9bdb431de94 Mon Sep 17 00:00:00 2001 From: Denys Fedoryshchenko Date: Tue, 11 Aug 2026 21:37:18 +0300 Subject: utils: centralize min macro Several userspace translation units carry identical local min() definitions. Move the guarded definition to utils.h and include it from each user so there is one implementation to maintain. The Linux min() macro lives in kernel-internal headers and is not part of the userspace UAPI. Clang/LLVM does not provide a compatible min macro either: C++ code uses std::min and Clang's similarly named operations use explicit builtin names. The userspace interface, where available, exposes uppercase MIN instead. Keep the #ifndef guard to preserve the behavior of the existing local definitions and avoid redefining a lowercase min macro supplied by an unrelated third-party header. --- accel-pppd/ctrl/pppoe/pppoe.c | 5 +---- accel-pppd/ctrl/sstp/sstp.c | 3 --- accel-pppd/log.c | 5 +---- accel-pppd/logs/log_pgsql.c | 3 +-- accel-pppd/ppp/ppp_lcp.c | 5 +---- accel-pppd/utils.h | 4 ++++ 6 files changed, 8 insertions(+), 17 deletions(-) (limited to 'accel-pppd/ctrl') diff --git a/accel-pppd/ctrl/pppoe/pppoe.c b/accel-pppd/ctrl/pppoe/pppoe.c index 72faab4e..0e65168d 100644 --- a/accel-pppd/ctrl/pppoe/pppoe.c +++ b/accel-pppd/ctrl/pppoe/pppoe.c @@ -24,6 +24,7 @@ #endif #include "iputils.h" +#include "utils.h" #include "connlimit.h" #include "vlan_mon.h" @@ -33,10 +34,6 @@ #define SID_MAX 65536 -#ifndef min -#define min(x,y) ((x)<(y)?(x):(y)) -#endif - struct pppoe_conn_t { struct list_head entry; struct triton_context_t ctx; diff --git a/accel-pppd/ctrl/sstp/sstp.c b/accel-pppd/ctrl/sstp/sstp.c index 2fd8cb35..72004cf8 100644 --- a/accel-pppd/ctrl/sstp/sstp.c +++ b/accel-pppd/ctrl/sstp/sstp.c @@ -47,9 +47,6 @@ #include "sstp_prot.h" #include "if_ppposeq.h" -#ifndef min -#define min(x,y) ((x) < (y) ? (x) : (y)) -#endif #ifndef max #define max(x,y) ((x) > (y) ? (x) : (y)) #endif diff --git a/accel-pppd/log.c b/accel-pppd/log.c index e4a1e6ed..7fac66df 100644 --- a/accel-pppd/log.c +++ b/accel-pppd/log.c @@ -11,15 +11,12 @@ #include "triton/mempool.h" #include "events.h" #include "ppp.h" +#include "utils.h" #include "log.h" #include "memdebug.h" -#ifndef min -#define min(x,y) ((x)<(y)?(x):(y)) -#endif - #define LOG_MSG 0 #define LOG_ERROR 1 #define LOG_WARN 2 diff --git a/accel-pppd/logs/log_pgsql.c b/accel-pppd/logs/log_pgsql.c index 99be5e64..5b3fd6ae 100644 --- a/accel-pppd/logs/log_pgsql.c +++ b/accel-pppd/logs/log_pgsql.c @@ -9,11 +9,10 @@ #include "log.h" #include "list.h" #include "ap_session.h" +#include "utils.h" #include "memdebug.h" -#define min(x,y) ((x)<(y)?(x):(y)) - static char *conf_conninfo; static int conf_queue_max = 1000; static char *conf_query; diff --git a/accel-pppd/ppp/ppp_lcp.c b/accel-pppd/ppp/ppp_lcp.c index 05b6c4a6..ed085b3b 100644 --- a/accel-pppd/ppp/ppp_lcp.c +++ b/accel-pppd/ppp/ppp_lcp.c @@ -13,13 +13,10 @@ #include "ppp_lcp.h" #include "events.h" #include "iputils.h" +#include "utils.h" #include "memdebug.h" -#ifndef min -#define min(x,y) ((x)<(y)?(x):(y)) -#endif - struct recv_opt_t { struct list_head entry; diff --git a/accel-pppd/utils.h b/accel-pppd/utils.h index aad4025f..63c1db0d 100644 --- a/accel-pppd/utils.h +++ b/accel-pppd/utils.h @@ -4,6 +4,10 @@ #include #include +#ifndef min +#define min(x, y) ((x) < (y) ? (x) : (y)) +#endif + char *u_ip6str(const struct in6_addr *addr, char *buf); char *u_ip4str(const struct in_addr *addr, char *buf); -- cgit v1.2.3