From 6c4593a87e689a83d7308efb0913a8dd9182051b Mon Sep 17 00:00:00 2001 From: Denys Fedoryshchenko Date: Tue, 1 Sep 2026 08:50:04 +0300 Subject: pppoe: decode tag integers alignment-safely Read peer-controlled PPPoE tag and TR-101 integer fields through aligned temporaries, validate PPP-Max-Payload before formatting, and avoid unaligned cookie timestamp accesses. --- accel-pppd/ctrl/pppoe/pppoe.c | 44 +++++++++++++++++++++++++++++++++++-------- accel-pppd/ctrl/pppoe/tr101.c | 36 +++++++++++++++++++++-------------- 2 files changed, 58 insertions(+), 22 deletions(-) diff --git a/accel-pppd/ctrl/pppoe/pppoe.c b/accel-pppd/ctrl/pppoe/pppoe.c index 0e65168d..00f962af 100644 --- a/accel-pppd/ctrl/pppoe/pppoe.c +++ b/accel-pppd/ctrl/pppoe/pppoe.c @@ -554,9 +554,30 @@ static void print_tag_octets(struct pppoe_tag *tag) log_info2("%02x", (uint8_t)tag->tag_data[i]); } +static uint16_t pppoe_read_u16(const void *ptr) +{ + uint16_t value; + + memcpy(&value, ptr, sizeof(value)); + return ntohs(value); +} + +static uint32_t pppoe_read_u32(const void *ptr) +{ + uint32_t value; + + memcpy(&value, ptr, sizeof(value)); + return ntohl(value); +} + static void print_tag_u16(struct pppoe_tag *tag) { - log_info2("%i", (uint16_t)ntohs(*(uint16_t *)tag->tag_data)); + if (ntohs(tag->tag_len) != sizeof(uint16_t)) { + log_info2("invalid"); + return; + } + + log_info2("%i", pppoe_read_u16(tag->tag_data)); } static void print_packet(const char *ifname, const char *op, uint8_t *pack) @@ -633,7 +654,7 @@ static void print_packet(const char *ifname, const char *op, uint8_t *pack) if (ntohs(tag->tag_len) < 4) log_info2(" "); else - log_info2(" ", ntohl(*(uint32_t *)tag->tag_data)); + log_info2(" ", pppoe_read_u32(tag->tag_data)); break; case TAG_RELAY_SESSION_ID: log_info2(" secret, SECRET_LENGTH); @@ -1073,7 +1101,7 @@ static void pppoe_recv_PADI(struct pppoe_serv_t *serv, uint8_t *pack, int size) break; case TAG_PPP_MAX_PAYLOAD: if (ntohs(tag->tag_len) == 2) - ppp_max_payload = ntohs(*(uint16_t *)tag->tag_data); + ppp_max_payload = pppoe_read_u16(tag->tag_data); break; } } @@ -1220,14 +1248,14 @@ static void pppoe_recv_PADR(struct pppoe_serv_t *serv, uint8_t *pack, int size) case TAG_VENDOR_SPECIFIC: if (ntohs(tag->tag_len) < 4) continue; - vendor_id = ntohl(*(uint32_t *)tag->tag_data); + vendor_id = pppoe_read_u32(tag->tag_data); if (vendor_id == VENDOR_ADSL_FORUM) if (conf_tr101) tr101_tag = tag; break; case TAG_PPP_MAX_PAYLOAD: if (ntohs(tag->tag_len) == 2) - ppp_max_payload = ntohs(*(uint16_t *)tag->tag_data); + ppp_max_payload = pppoe_read_u16(tag->tag_data); break; } } diff --git a/accel-pppd/ctrl/pppoe/tr101.c b/accel-pppd/ctrl/pppoe/tr101.c index bb8b845a..e7aa96dc 100644 --- a/accel-pppd/ctrl/pppoe/tr101.c +++ b/accel-pppd/ctrl/pppoe/tr101.c @@ -30,6 +30,14 @@ #define ACCESS_LOOP_ENCAP 0x90 #define IFW_SESSION 0xFE +static uint32_t tr101_read_u32(const void *ptr) +{ + uint32_t value; + + memcpy(&value, ptr, sizeof(value)); + return ntohl(value); +} + static int tr101_send_request(struct pppoe_tag *tr101, struct rad_packet_t *pack, int type) { uint8_t *ptr = (uint8_t *)tr101->tag_data + 4; @@ -75,85 +83,85 @@ static int tr101_send_request(struct pppoe_tag *tr101, struct rad_packet_t *pack case OPT_ACTUAL_DATA_RATE_UP: if (len != 4) goto inval; - if (rad_packet_add_int(pack, "ADSL-Forum", "Actual-Data-Rate-Upstream", ntohl(*(uint32_t *)ptr))) + if (rad_packet_add_int(pack, "ADSL-Forum", "Actual-Data-Rate-Upstream", tr101_read_u32(ptr))) return -1; break; case OPT_ACTUAL_DATA_RATE_DOWN: if (len != 4) goto inval; - if (rad_packet_add_int(pack, "ADSL-Forum", "Actual-Data-Rate-Downstream", ntohl(*(uint32_t *)ptr))) + if (rad_packet_add_int(pack, "ADSL-Forum", "Actual-Data-Rate-Downstream", tr101_read_u32(ptr))) return -1; break; case OPT_MIN_DATA_RATE_UP: if (len != 4) goto inval; - if (rad_packet_add_int(pack, "ADSL-Forum", "Minimum-Data-Rate-Upstream", ntohl(*(uint32_t *)ptr))) + if (rad_packet_add_int(pack, "ADSL-Forum", "Minimum-Data-Rate-Upstream", tr101_read_u32(ptr))) return -1; break; case OPT_MIN_DATA_RATE_DOWN: if (len != 4) goto inval; - if (rad_packet_add_int(pack, "ADSL-Forum", "Minimum-Data-Rate-Downstream", ntohl(*(uint32_t *)ptr))) + if (rad_packet_add_int(pack, "ADSL-Forum", "Minimum-Data-Rate-Downstream", tr101_read_u32(ptr))) return -1; break; case OPT_ATT_DATA_RATE_UP: if (len != 4) goto inval; - if (rad_packet_add_int(pack, "ADSL-Forum", "Attainable-Data-Rate-Upstream", ntohl(*(uint32_t *)ptr))) + if (rad_packet_add_int(pack, "ADSL-Forum", "Attainable-Data-Rate-Upstream", tr101_read_u32(ptr))) return -1; break; case OPT_ATT_DATA_RATE_DOWN: if (len != 4) goto inval; - if (rad_packet_add_int(pack, "ADSL-Forum", "Attainable-Data-Rate-Downstream", ntohl(*(uint32_t *)ptr))) + if (rad_packet_add_int(pack, "ADSL-Forum", "Attainable-Data-Rate-Downstream", tr101_read_u32(ptr))) return -1; break; case OPT_MAX_DATA_RATE_UP: if (len != 4) goto inval; - if (rad_packet_add_int(pack, "ADSL-Forum", "Maximum-Data-Rate-Upstream", ntohl(*(uint32_t *)ptr))) + if (rad_packet_add_int(pack, "ADSL-Forum", "Maximum-Data-Rate-Upstream", tr101_read_u32(ptr))) return -1; break; case OPT_MAX_DATA_RATE_DOWN: if (len != 4) goto inval; - if (rad_packet_add_int(pack, "ADSL-Forum", "Maximum-Data-Rate-Downstream", ntohl(*(uint32_t *)ptr))) + if (rad_packet_add_int(pack, "ADSL-Forum", "Maximum-Data-Rate-Downstream", tr101_read_u32(ptr))) return -1; break; case OPT_MIN_DATA_RATE_UP_LP: if (len != 4) goto inval; - if (rad_packet_add_int(pack, "ADSL-Forum", "Minimum-Data-Rate-Upstream-Low-Power", ntohl(*(uint32_t *)ptr))) + if (rad_packet_add_int(pack, "ADSL-Forum", "Minimum-Data-Rate-Upstream-Low-Power", tr101_read_u32(ptr))) return -1; break; case OPT_MIN_DATA_RATE_DOWN_LP: if (len != 4) goto inval; - if (rad_packet_add_int(pack, "ADSL-Forum", "Minimum-Data-Rate-Downstream-Low-Power", ntohl(*(uint32_t *)ptr))) + if (rad_packet_add_int(pack, "ADSL-Forum", "Minimum-Data-Rate-Downstream-Low-Power", tr101_read_u32(ptr))) return -1; break; case OPT_MAX_INTERL_DELAY_UP: if (len != 4) goto inval; - if (rad_packet_add_int(pack, "ADSL-Forum", "Maximum-Interleaving-Delay-Upstream", ntohl(*(uint32_t *)ptr))) + if (rad_packet_add_int(pack, "ADSL-Forum", "Maximum-Interleaving-Delay-Upstream", tr101_read_u32(ptr))) return -1; break; case OPT_ACTUAL_INTERL_DELAY_UP: if (len != 4) goto inval; - if (rad_packet_add_int(pack, "ADSL-Forum", "Actual-Interleaving-Delay-Upstream", ntohl(*(uint32_t *)ptr))) + if (rad_packet_add_int(pack, "ADSL-Forum", "Actual-Interleaving-Delay-Upstream", tr101_read_u32(ptr))) return -1; break; case OPT_MAX_INTER_DELAY_DOWN: if (len != 4) goto inval; - if (rad_packet_add_int(pack, "ADSL-Forum", "Maximum-Interleaving-Delay-Downstream", ntohl(*(uint32_t *)ptr))) + if (rad_packet_add_int(pack, "ADSL-Forum", "Maximum-Interleaving-Delay-Downstream", tr101_read_u32(ptr))) return -1; break; case OPT_ACTUAL_INTER_DELAY_DOWN: if (len != 4) goto inval; - if (rad_packet_add_int(pack, "ADSL-Forum", "Actual-Interleaving-Delay-Downstream", ntohl(*(uint32_t *)ptr))) + if (rad_packet_add_int(pack, "ADSL-Forum", "Actual-Interleaving-Delay-Downstream", tr101_read_u32(ptr))) return -1; break; case ACCESS_LOOP_ENCAP: -- cgit v1.2.3