From 81f3ca5308f76f9894422e5e1d5113b55002b66e Mon Sep 17 00:00:00 2001 From: Marius Tomaschewski Date: Fri, 13 Mar 2026 11:08:58 +0100 Subject: [PATCH 1/5] ospfd: Backport TLV length checks Upstream: yes References: CVE-2025-61099,CVE-2025-61100,CVE-2025-61101,CVE-2025-61102,CVE-2025-61103,CVE-2025-61104,CVE-2025-61105,CVE-2025-61106,CVE-2025-61107,bsc#1252838,bsc#1252829,bsc#1252833,bsc#1252835,bsc#1252810,bsc#1252811,bsc#1252761,bsc#1252812,bsc#1252813 Backport from commit 8db278b5e3e2b1a8b2d8ac85789565d5dd268ac6 in https://github.com/FRRouting/frr/pull/8408 ``` ospfd: Correct Coverity defects When browsing or parsing OSPF LSA TLVs, we need to use the LSA length which is part of the LSA header. This length, encoded in 16 bits, must be first converted to host byte order with ntohs() function. However, Coverity Scan considers that ntohs() function return TAINTED data. Thus, when the length is used to control for() loop, Coverity Scan marks this part of the code as defect with "Untrusted Loop Bound" due to the usage of Tainted variable. Similar problems occur when browsing sub-TLV where length is extracted with ntohs(). To overcome this limitation, a size attribute has been added to the ospf_lsa structure. The size is set when lsa->data buffer is allocated. In addition, when an OSPF packet is received, the size of the payload is controlled before contains is processed. For OSPF LSA, this allow a secure buffer allocation. Thus, new size attribute contains the exact buffer allocation allowing a strict control during TLV browsing. This patch adds extra control to bound for() loop during TLV browsing to avoid potential problem as suggested by Coverity Scan. Controls are based on new size attribute of the ospf_lsa structure to avoid any ambiguity. Signed-off-by: Olivier Dugeon ``` Signed-off-by: Marius Tomaschewski diff --git a/ospfd/ospf_api.c b/ospfd/ospf_api.c index 7e7236a3b6..81de882754 100644 --- a/ospfd/ospf_api.c +++ b/ospfd/ospf_api.c @@ -58,7 +58,7 @@ /* For debugging only, will be removed */ -void api_opaque_lsa_print(struct lsa_header *data) +void api_opaque_lsa_print(struct ospf_lsa *lsa) { struct opaque_lsa { struct lsa_header header; @@ -69,11 +69,11 @@ void api_opaque_lsa_print(struct lsa_header *data) int opaquelen; int i; - ospf_lsa_header_dump(data); + ospf_lsa_header_dump(lsa->data); - olsa = (struct opaque_lsa *)data; + olsa = (struct opaque_lsa *)lsa->data; - opaquelen = ntohs(data->length) - OSPF_LSA_HEADER_SIZE; + opaquelen = lsa->size - OSPF_LSA_HEADER_SIZE; zlog_debug("apiserver_lsa_print: opaquelen=%d", opaquelen); for (i = 0; i < opaquelen; i++) { @@ -111,11 +111,16 @@ struct msg *msg_new(uint8_t msgtype, void *msgbody, uint32_t seqnum, struct msg *msg_dup(struct msg *msg) { struct msg *new; + size_t size; assert(msg); + size = ntohs(msg->hdr.msglen); + if (size > OSPF_MAX_LSA_SIZE) + return NULL; + new = msg_new(msg->hdr.msgtype, STREAM_DATA(msg->s), - ntohl(msg->hdr.msgseq), ntohs(msg->hdr.msglen)); + ntohl(msg->hdr.msgseq), size); return new; } @@ -400,7 +405,7 @@ struct msg *msg_read(int fd) } /* Allocate new message */ - msg = msg_new(hdr.msgtype, buf, ntohl(hdr.msgseq), ntohs(hdr.msglen)); + msg = msg_new(hdr.msgtype, buf, ntohl(hdr.msgseq), bodylen); return msg; } @@ -408,29 +413,34 @@ struct msg *msg_read(int fd) int msg_write(int fd, struct msg *msg) { uint8_t buf[OSPF_API_MAX_MSG_SIZE]; - int l; + uint16_t l; int wlen; assert(msg); assert(msg->s); - /* Length of message including header */ - l = sizeof(struct apimsghdr) + ntohs(msg->hdr.msglen); + /* Length of OSPF LSA payload */ + l = ntohs(msg->hdr.msglen); + if (l > OSPF_MAX_LSA_SIZE) { + zlog_warn("%s: wrong LSA size %d", __func__, l); + return -1; + } /* Make contiguous memory buffer for message */ memcpy(buf, &msg->hdr, sizeof(struct apimsghdr)); - memcpy(buf + sizeof(struct apimsghdr), STREAM_DATA(msg->s), - ntohs(msg->hdr.msglen)); + memcpy(buf + sizeof(struct apimsghdr), STREAM_DATA(msg->s), l); + /* Total length of OSPF API Message */ + l += sizeof(struct apimsghdr); wlen = writen(fd, buf, l); if (wlen < 0) { - zlog_warn("msg_write: writen %s", safe_strerror(errno)); + zlog_warn("%s: writen %s", __func__, safe_strerror(errno)); return -1; } else if (wlen == 0) { - zlog_warn("msg_write: Connection closed by peer"); + zlog_warn("%s: Connection closed by peer", __func__); return -1; } else if (wlen != l) { - zlog_warn("msg_write: Cannot write API message"); + zlog_warn("%s: Cannot write API message", __func__); return -1; } return 0; diff --git a/ospfd/ospf_api.h b/ospfd/ospf_api.h index 0fc683a5db..c20284aed5 100644 --- a/ospfd/ospf_api.h +++ b/ospfd/ospf_api.h @@ -276,7 +276,7 @@ struct apimsg { */ /* For debugging only. */ -extern void api_opaque_lsa_print(struct lsa_header *data); +extern void api_opaque_lsa_print(struct ospf_lsa *lsa); /* Messages sent by client */ extern struct msg *new_msg_register_opaque_type(uint32_t seqnum, uint8_t ltype, diff --git a/ospfd/ospf_apiserver.c b/ospfd/ospf_apiserver.c index 49730063b9..45e9f9dacd 100644 --- a/ospfd/ospf_apiserver.c +++ b/ospfd/ospf_apiserver.c @@ -1167,6 +1167,7 @@ int ospf_apiserver_handle_register_event(struct ospf_apiserver *apiserv, struct msg_register_event *rmsg; int rc; uint32_t seqnum; + size_t size; rmsg = (struct msg_register_event *)STREAM_DATA(msg->s); @@ -1176,13 +1177,16 @@ int ospf_apiserver_handle_register_event(struct ospf_apiserver *apiserv, /* Free existing filter in apiserv. */ XFREE(MTYPE_OSPF_APISERVER_MSGFILTER, apiserv->filter); /* Alloc new space for filter. */ + size = ntohs(msg->hdr.msglen); + if (size < OSPF_MAX_LSA_SIZE) { - apiserv->filter = - XMALLOC(MTYPE_OSPF_APISERVER_MSGFILTER, ntohs(msg->hdr.msglen)); + apiserv->filter = XMALLOC(MTYPE_OSPF_APISERVER_MSGFILTER, size); - /* copy it over. */ - memcpy(apiserv->filter, &rmsg->filter, ntohs(msg->hdr.msglen)); - rc = OSPF_API_OK; + /* copy it over. */ + memcpy(apiserv->filter, &rmsg->filter, size); + rc = OSPF_API_OK; + } else + rc = OSPF_API_NOMEMORY; /* Send a reply back to client with return code */ rc = ospf_apiserver_send_reply(apiserv, seqnum, rc); diff --git a/ospfd/ospf_dump.c b/ospfd/ospf_dump.c index 3dcb2b481d..2b3bb32713 100644 --- a/ospfd/ospf_dump.c +++ b/ospfd/ospf_dump.c @@ -228,7 +228,7 @@ const char *ospf_timer_dump(struct thread *t, char *buf, size_t size) static void ospf_packet_hello_dump(struct stream *s, uint16_t length) { struct ospf_hello *hello; - int i; + int i, len; hello = (struct ospf_hello *)stream_pnt(s); @@ -243,9 +243,9 @@ static void ospf_packet_hello_dump(struct stream *s, uint16_t length) zlog_debug(" DRouter %s", inet_ntoa(hello->d_router)); zlog_debug(" BDRouter %s", inet_ntoa(hello->bd_router)); - length -= OSPF_HEADER_SIZE + OSPF_HELLO_MIN_SIZE; - zlog_debug(" # Neighbors %d", length / 4); - for (i = 0; length > 0; i++, length -= sizeof(struct in_addr)) + len = length - OSPF_HEADER_SIZE - OSPF_HELLO_MIN_SIZE; + zlog_debug(" # Neighbors %d", len / 4); + for (i = 0; len > 0; i++, len -= sizeof(struct in_addr)) zlog_debug(" Neighbor %s", inet_ntoa(hello->neighbors[i])); } @@ -272,7 +272,8 @@ static void ospf_router_lsa_dump(struct stream *s, uint16_t length) { char buf[BUFSIZ]; struct router_lsa *rl; - int i, len; + struct router_link *rlnk; + int i, len, sum; rl = (struct router_lsa *)stream_pnt(s); @@ -281,16 +282,15 @@ static void ospf_router_lsa_dump(struct stream *s, uint16_t length) ospf_router_lsa_flags_dump(rl->flags, buf, BUFSIZ)); zlog_debug(" # links %d", ntohs(rl->links)); - len = ntohs(rl->header.length) - OSPF_LSA_HEADER_SIZE - 4; - for (i = 0; len > 0; i++) { - zlog_debug(" Link ID %s", inet_ntoa(rl->link[i].link_id)); - zlog_debug(" Link Data %s", - inet_ntoa(rl->link[i].link_data)); - zlog_debug(" Type %d", (uint8_t)rl->link[i].type); - zlog_debug(" TOS %d", (uint8_t)rl->link[i].tos); - zlog_debug(" metric %d", ntohs(rl->link[i].metric)); - - len -= 12; + len = length - OSPF_LSA_HEADER_SIZE - 4; + rlnk = &rl->link[0]; + sum = 0; + for (i = 0; sum < len && rlnk; sum += 12, rlnk = &rl->link[++i]) { + zlog_debug(" Link ID %pI4", &rlnk->link_id); + zlog_debug(" Link Data %pI4", &rlnk->link_data); + zlog_debug(" Type %d", (uint8_t)rlnk->type); + zlog_debug(" TOS %d", (uint8_t)rlnk->tos); + zlog_debug(" metric %d", ntohs(rlnk->metric)); } } @@ -299,10 +299,11 @@ static void ospf_network_lsa_dump(struct stream *s, uint16_t length) struct network_lsa *nl; int i, cnt; + zlog_debug(" Network-LSA"); + nl = (struct network_lsa *)stream_pnt(s); - cnt = (ntohs(nl->header.length) - (OSPF_LSA_HEADER_SIZE + 4)) / 4; + cnt = (length - (OSPF_LSA_HEADER_SIZE + 4)) / 4; - zlog_debug(" Network-LSA"); /* zlog_debug ("LSA total size %d", ntohs (nl->header.length)); zlog_debug ("Network-LSA size %d", @@ -318,55 +319,53 @@ static void ospf_network_lsa_dump(struct stream *s, uint16_t length) static void ospf_summary_lsa_dump(struct stream *s, uint16_t length) { struct summary_lsa *sl; - int size; - int i; sl = (struct summary_lsa *)stream_pnt(s); zlog_debug(" Summary-LSA"); zlog_debug(" Network Mask %s", inet_ntoa(sl->mask)); - - size = ntohs(sl->header.length) - OSPF_LSA_HEADER_SIZE - 4; - for (i = 0; size > 0; size -= 4, i++) - zlog_debug(" TOS=%d metric %d", sl->tos, - GET_METRIC(sl->metric)); + zlog_debug(" TOS=%d metric %d", sl->tos, GET_METRIC(sl->metric)); } static void ospf_as_external_lsa_dump(struct stream *s, uint16_t length) { struct as_external_lsa *al; - int size; + struct as_route *asr; + int size, sum; int i; al = (struct as_external_lsa *)stream_pnt(s); zlog_debug(" %s", ospf_lsa_type_msg[al->header.type].str); zlog_debug(" Network Mask %s", inet_ntoa(al->mask)); - size = ntohs(al->header.length) - OSPF_LSA_HEADER_SIZE - 4; - for (i = 0; size > 0; size -= 12, i++) { + size = length - OSPF_LSA_HEADER_SIZE - 4; + asr = &al->e[0]; + sum = 0; + for (i = 0; sum < size && asr; sum += 12, asr = &al->e[++i]) { zlog_debug(" bit %s TOS=%d metric %d", - IS_EXTERNAL_METRIC(al->e[i].tos) ? "E" : "-", - al->e[i].tos & 0x7f, GET_METRIC(al->e[i].metric)); - zlog_debug(" Forwarding address %s", - inet_ntoa(al->e[i].fwd_addr)); + IS_EXTERNAL_METRIC(asr->tos) ? "E" : "-", + asr->tos & 0x7f, GET_METRIC(asr->metric)); + zlog_debug(" Forwarding address %pI4", &asr->fwd_addr); zlog_debug(" External Route Tag %" ROUTE_TAG_PRI, - al->e[i].route_tag); + asr->route_tag); } } static void ospf_lsa_header_list_dump(struct stream *s, uint16_t length) { struct lsa_header *lsa; + int len; zlog_debug(" # LSA Headers %d", length / OSPF_LSA_HEADER_SIZE); /* LSA Headers. */ - while (length > 0) { + len = length; + while (len > 0) { lsa = (struct lsa_header *)stream_pnt(s); ospf_lsa_header_dump(lsa); stream_forward_getp(s, OSPF_LSA_HEADER_SIZE); - length -= OSPF_LSA_HEADER_SIZE; + len -= OSPF_LSA_HEADER_SIZE; } } @@ -404,6 +403,7 @@ static void ospf_packet_ls_req_dump(struct stream *s, uint16_t length) uint32_t ls_type; struct in_addr ls_id; struct in_addr adv_router; + int sum; sp = stream_get_getp(s); @@ -412,7 +412,8 @@ static void ospf_packet_ls_req_dump(struct stream *s, uint16_t length) zlog_debug("Link State Request"); zlog_debug(" # Requests %d", length / 12); - for (; length > 0; length -= 12) { + sum = 0; + for (; sum < length; sum += 12) { ls_type = stream_getl(s); ls_id.s_addr = stream_get_ipv4(s); adv_router.s_addr = stream_get_ipv4(s); @@ -429,23 +430,23 @@ static void ospf_packet_ls_upd_dump(struct stream *s, uint16_t length) { uint32_t sp; struct lsa_header *lsa; - int lsa_len; + int lsa_len, len; uint32_t count; - length -= OSPF_HEADER_SIZE; + len = length - OSPF_HEADER_SIZE; sp = stream_get_getp(s); count = stream_getl(s); - length -= 4; + len -= 4; zlog_debug("Link State Update"); zlog_debug(" # LSAs %d", count); - while (length > 0 && count > 0) { - if (length < OSPF_HEADER_SIZE || length % 4 != 0) { + while (len > 0 && count > 0) { + if ((uint16_t)len < OSPF_LSA_HEADER_SIZE || len % 4 != 0) { zlog_debug(" Remaining %d bytes; Incorrect length.", - length); + len); break; } @@ -453,34 +454,39 @@ static void ospf_packet_ls_upd_dump(struct stream *s, uint16_t length) lsa_len = ntohs(lsa->length); ospf_lsa_header_dump(lsa); + /* Check that LSA length is valid */ + if (lsa_len > len || lsa_len % 4 != 0) { + zlog_debug(" LSA length %d is incorrect!", lsa_len); + break; + } switch (lsa->type) { case OSPF_ROUTER_LSA: - ospf_router_lsa_dump(s, length); + ospf_router_lsa_dump(s, lsa_len); break; case OSPF_NETWORK_LSA: - ospf_network_lsa_dump(s, length); + ospf_network_lsa_dump(s, lsa_len); break; case OSPF_SUMMARY_LSA: case OSPF_ASBR_SUMMARY_LSA: - ospf_summary_lsa_dump(s, length); + ospf_summary_lsa_dump(s, lsa_len); break; case OSPF_AS_EXTERNAL_LSA: - ospf_as_external_lsa_dump(s, length); + ospf_as_external_lsa_dump(s, lsa_len); break; case OSPF_AS_NSSA_LSA: - ospf_as_external_lsa_dump(s, length); + ospf_as_external_lsa_dump(s, lsa_len); break; case OSPF_OPAQUE_LINK_LSA: case OSPF_OPAQUE_AREA_LSA: case OSPF_OPAQUE_AS_LSA: - ospf_opaque_lsa_dump(s, length); + ospf_opaque_lsa_dump(s, lsa_len); break; default: break; } stream_forward_getp(s, lsa_len); - length -= lsa_len; + len -= lsa_len; count--; } diff --git a/ospfd/ospf_ext.c b/ospfd/ospf_ext.c index 47883d5f39..ff14087ff7 100644 --- a/ospfd/ospf_ext.c +++ b/ospfd/ospf_ext.c @@ -1579,13 +1579,23 @@ static void ospf_ext_lsa_schedule(struct ext_itf *exti, enum lsa_opcode op) * ------------------------------------ */ +#define check_tlv_size(size, msg) \ + do { \ + if (ntohs(tlvh->length) != size) { \ + vty_out(vty, " Wrong %s TLV size: %d(%d). Abort!\n", \ + msg, ntohs(tlvh->length), size); \ + return size + TLV_HDR_SIZE; \ + } \ + } while (0) + /* Cisco experimental SubTLV */ static uint16_t show_vty_ext_link_rmt_itf_addr(struct vty *vty, struct tlv_header *tlvh) { - struct ext_subtlv_rmt_itf_addr *top; + struct ext_subtlv_rmt_itf_addr *top = + (struct ext_subtlv_rmt_itf_addr *)tlvh; - top = (struct ext_subtlv_rmt_itf_addr *)tlvh; + check_tlv_size(EXT_SUBTLV_RMT_ITF_ADDR_SIZE, "Remote Itf. Address"); vty_out(vty, " Remote Interface Address Sub-TLV: Length %u\n " @@ -1601,6 +1611,8 @@ static uint16_t show_vty_ext_link_adj_sid(struct vty *vty, { struct ext_subtlv_adj_sid *top = (struct ext_subtlv_adj_sid *)tlvh; + check_tlv_size(EXT_SUBTLV_ADJ_SID_SIZE, "Adjacency SID"); + vty_out(vty, " Adj-SID Sub-TLV: Length %u\n\tFlags: " "0x%x\n\tMT-ID:0x%x\n\tWeight: 0x%x\n\t%s: %u\n", @@ -1621,6 +1633,8 @@ static uint16_t show_vty_ext_link_lan_adj_sid(struct vty *vty, struct ext_subtlv_lan_adj_sid *top = (struct ext_subtlv_lan_adj_sid *)tlvh; + check_tlv_size(EXT_SUBTLV_LAN_ADJ_SID_SIZE, "Lan-Adjacency SID"); + vty_out(vty, " LAN-Adj-SID Sub-TLV: Length %u\n\tFlags: " "0x%x\n\tMT-ID:0x%x\n\tWeight: 0x%x\n\tNeighbor ID: " @@ -1636,8 +1650,15 @@ static uint16_t show_vty_ext_link_lan_adj_sid(struct vty *vty, return TLV_SIZE(tlvh); } -static uint16_t show_vty_unknown_tlv(struct vty *vty, struct tlv_header *tlvh) +static uint16_t show_vty_unknown_tlv(struct vty *vty, struct tlv_header *tlvh, + size_t buf_size) { + if (TLV_SIZE(tlvh) > buf_size) { + vty_out(vty, " TLV size %d exceeds buffer size. Abort!", + TLV_SIZE(tlvh)); + return buf_size; + } + vty_out(vty, " Unknown TLV: [type(0x%x), length(0x%x)]\n", ntohs(tlvh->type), ntohs(tlvh->length)); @@ -1645,13 +1666,22 @@ static uint16_t show_vty_unknown_tlv(struct vty *vty, struct tlv_header *tlvh) } /* Extended Link Sub TLVs */ -static uint16_t show_vty_link_info(struct vty *vty, struct tlv_header *ext) +static uint16_t show_vty_link_info(struct vty *vty, struct tlv_header *ext, + size_t buf_size) { struct ext_tlv_link *top = (struct ext_tlv_link *)ext; struct tlv_header *tlvh; - uint16_t length = ntohs(top->header.length) - 3 * sizeof(uint32_t); + uint16_t length = ntohs(top->header.length); uint16_t sum = 0; + /* Verify that TLV length is valid against remaining buffer size */ + if (length > buf_size) { + vty_out(vty, + " Extended Link TLV size %d exceeds buffer size. Abort!\n", + length); + return buf_size; + } + vty_out(vty, " Extended Link TLV: Length %u\n Link Type: 0x%x\n" " Link ID: %s\n", @@ -1659,9 +1689,11 @@ static uint16_t show_vty_link_info(struct vty *vty, struct tlv_header *ext) inet_ntoa(top->link_id)); vty_out(vty, " Link data: %s\n", inet_ntoa(top->link_data)); + /* Skip Extended TLV and parse sub-TLVs */ + length -= EXT_TLV_LINK_SIZE; tlvh = (struct tlv_header *)((char *)(ext) + TLV_HDR_SIZE + EXT_TLV_LINK_SIZE); - for (; sum < length; tlvh = TLV_HDR_NEXT(tlvh)) { + for (; sum < length && tlvh; tlvh = TLV_HDR_NEXT(tlvh)) { switch (ntohs(tlvh->type)) { case EXT_SUBTLV_ADJ_SID: sum += show_vty_ext_link_adj_sid(vty, tlvh); @@ -1673,7 +1705,7 @@ static uint16_t show_vty_link_info(struct vty *vty, struct tlv_header *ext) sum += show_vty_ext_link_rmt_itf_addr(vty, tlvh); break; default: - sum += show_vty_unknown_tlv(vty, tlvh); + sum += show_vty_unknown_tlv(vty, tlvh, length - sum); break; } } @@ -1689,16 +1721,16 @@ static void ospf_ext_link_show_info(struct vty *vty, struct ospf_lsa *lsa) uint16_t length = 0, sum = 0; /* Initialize TLV browsing */ - length = ntohs(lsah->length) - OSPF_LSA_HEADER_SIZE; + length = lsa->size - OSPF_LSA_HEADER_SIZE; - for (tlvh = TLV_HDR_TOP(lsah); sum < length; + for (tlvh = TLV_HDR_TOP(lsah); sum < length && tlvh; tlvh = TLV_HDR_NEXT(tlvh)) { switch (ntohs(tlvh->type)) { case EXT_TLV_LINK: - sum += show_vty_link_info(vty, tlvh); + sum += show_vty_link_info(vty, tlvh, length - sum); break; default: - sum += show_vty_unknown_tlv(vty, tlvh); + sum += show_vty_unknown_tlv(vty, tlvh, length - sum); break; } } @@ -1711,6 +1743,8 @@ static uint16_t show_vty_ext_pref_pref_sid(struct vty *vty, struct ext_subtlv_prefix_sid *top = (struct ext_subtlv_prefix_sid *)tlvh; + check_tlv_size(EXT_SUBTLV_PREFIX_SID_SIZE, "Prefix SID"); + vty_out(vty, " Prefix SID Sub-TLV: Length %u\n\tAlgorithm: " "%u\n\tFlags: 0x%x\n\tMT-ID:0x%x\n\t%s: %u\n", @@ -1726,28 +1760,39 @@ static uint16_t show_vty_ext_pref_pref_sid(struct vty *vty, } /* Extended Prefix SubTLVs */ -static uint16_t show_vty_pref_info(struct vty *vty, struct tlv_header *ext) +static uint16_t show_vty_pref_info(struct vty *vty, struct tlv_header *ext, + size_t buf_size) { struct ext_tlv_prefix *top = (struct ext_tlv_prefix *)ext; struct tlv_header *tlvh; - uint16_t length = ntohs(top->header.length) - 2 * sizeof(uint32_t); + uint16_t length = ntohs(top->header.length); uint16_t sum = 0; + /* Verify that TLV length is valid against remaining buffer size */ + if (length > buf_size) { + vty_out(vty, + " Extended Link TLV size %d exceeds buffer size. Abort!\n", + length); + return buf_size; + } + vty_out(vty, " Extended Prefix TLV: Length %u\n\tRoute Type: %u\n" "\tAddress Family: 0x%x\n\tFlags: 0x%x\n\tAddress: %s/%u\n", ntohs(top->header.length), top->route_type, top->af, top->flags, inet_ntoa(top->address), top->pref_length); + /* Skip Extended Prefix TLV and parse sub-TLVs */ + length -= EXT_TLV_PREFIX_SIZE; tlvh = (struct tlv_header *)((char *)(ext) + TLV_HDR_SIZE + EXT_TLV_PREFIX_SIZE); - for (; sum < length; tlvh = TLV_HDR_NEXT(tlvh)) { + for (; sum < length && tlvh; tlvh = TLV_HDR_NEXT(tlvh)) { switch (ntohs(tlvh->type)) { case EXT_SUBTLV_PREFIX_SID: sum += show_vty_ext_pref_pref_sid(vty, tlvh); break; default: - sum += show_vty_unknown_tlv(vty, tlvh); + sum += show_vty_unknown_tlv(vty, tlvh, length - sum); break; } } @@ -1763,16 +1808,16 @@ static void ospf_ext_pref_show_info(struct vty *vty, struct ospf_lsa *lsa) uint16_t length = 0, sum = 0; /* Initialize TLV browsing */ - length = ntohs(lsah->length) - OSPF_LSA_HEADER_SIZE; + length = lsa->size - OSPF_LSA_HEADER_SIZE; - for (tlvh = TLV_HDR_TOP(lsah); sum < length; + for (tlvh = TLV_HDR_TOP(lsah); sum < length && tlvh; tlvh = TLV_HDR_NEXT(tlvh)) { switch (ntohs(tlvh->type)) { case EXT_TLV_PREFIX: - sum += show_vty_pref_info(vty, tlvh); + sum += show_vty_pref_info(vty, tlvh, length - sum); break; default: - sum += show_vty_unknown_tlv(vty, tlvh); + sum += show_vty_unknown_tlv(vty, tlvh, length - sum); break; } } diff --git a/ospfd/ospf_lsa.c b/ospfd/ospf_lsa.c index d089ea76cd..8c9f8e84fd 100644 --- a/ospfd/ospf_lsa.c +++ b/ospfd/ospf_lsa.c @@ -174,6 +174,7 @@ struct ospf_lsa *ospf_lsa_new_and_data(size_t size) new = ospf_lsa_new(); new->data = ospf_lsa_data_new(size); + new->size = size; return new; } @@ -3182,22 +3183,22 @@ int ospf_lsa_different(struct ospf_lsa *l1, struct ospf_lsa *l2) if (IS_LSA_MAXAGE(l2) && !IS_LSA_MAXAGE(l1)) return 1; - if (l1->data->length != l2->data->length) + if (l1->size != l2->size) return 1; - if (l1->data->length == 0) + if (l1->size == 0) return 1; if (CHECK_FLAG((l1->flags ^ l2->flags), OSPF_LSA_RECEIVED)) return 1; /* May be a stale LSA in the LSBD */ - assert(ntohs(l1->data->length) > OSPF_LSA_HEADER_SIZE); + assert(l1->size > OSPF_LSA_HEADER_SIZE); p1 = (char *)l1->data; p2 = (char *)l2->data; if (memcmp(p1 + OSPF_LSA_HEADER_SIZE, p2 + OSPF_LSA_HEADER_SIZE, - ntohs(l1->data->length) - OSPF_LSA_HEADER_SIZE) + l1->size - OSPF_LSA_HEADER_SIZE) != 0) return 1; diff --git a/ospfd/ospf_lsa.h b/ospfd/ospf_lsa.h index 4033659bff..58483d0b4b 100644 --- a/ospfd/ospf_lsa.h +++ b/ospfd/ospf_lsa.h @@ -84,8 +84,9 @@ struct ospf_lsa { #define OSPF_LSA_PREMATURE_AGE 0x40 #define OSPF_LSA_IN_MAXAGE 0x80 - /* LSA data. */ + /* LSA data. and size */ struct lsa_header *data; + size_t size; /* Received time stamp. */ struct timeval tv_recv; @@ -165,7 +166,7 @@ struct router_lsa { uint8_t flags; uint8_t zero; uint16_t links; - struct { + struct router_link { struct in_addr link_id; struct in_addr link_data; uint8_t type; @@ -196,7 +197,7 @@ struct summary_lsa { struct as_external_lsa { struct lsa_header header; struct in_addr mask; - struct { + struct as_route { uint8_t tos; uint8_t metric[3]; struct in_addr fwd_addr; diff --git a/ospfd/ospf_opaque.c b/ospfd/ospf_opaque.c index 35fa5da74b..5b17822e27 100644 --- a/ospfd/ospf_opaque.c +++ b/ospfd/ospf_opaque.c @@ -1197,9 +1197,10 @@ void show_opaque_info_detail(struct vty *vty, struct ospf_lsa *lsa) void ospf_opaque_lsa_dump(struct stream *s, uint16_t length) { - struct ospf_lsa lsa; + struct ospf_lsa lsa = {}; lsa.data = (struct lsa_header *)stream_pnt(s); + lsa.size = length; show_opaque_info_detail(NULL, &lsa); return; } diff --git a/ospfd/ospf_opaque.h b/ospfd/ospf_opaque.h index 96155608b2..b8c04e6851 100644 --- a/ospfd/ospf_opaque.h +++ b/ospfd/ospf_opaque.h @@ -78,6 +78,7 @@ #define VALID_OPAQUE_INFO_LEN(lsahdr) \ ((ntohs((lsahdr)->length) >= sizeof(struct lsa_header)) \ + && ((ntohs((lsahdr)->length) < OSPF_MAX_LSA_SIZE)) \ && ((ntohs((lsahdr)->length) % sizeof(uint32_t)) == 0)) /* @@ -93,7 +94,7 @@ struct tlv_header { #define TLV_BODY_SIZE(tlvh) (ROUNDUP(ntohs((tlvh)->length), sizeof(uint32_t))) -#define TLV_SIZE(tlvh) (TLV_HDR_SIZE + TLV_BODY_SIZE(tlvh)) +#define TLV_SIZE(tlvh) (uint32_t)(TLV_HDR_SIZE + TLV_BODY_SIZE(tlvh)) #define TLV_HDR_TOP(lsah) \ (struct tlv_header *)((char *)(lsah) + OSPF_LSA_HEADER_SIZE) diff --git a/ospfd/ospf_ri.c b/ospfd/ospf_ri.c index c3d53ad5ed..2589b99fef 100644 --- a/ospfd/ospf_ri.c +++ b/ospfd/ospf_ri.c @@ -286,8 +286,8 @@ static void set_pce_address(struct in_addr ipv4, struct ospf_pce_info *pce) pce->pce_header.header.type = htons(RI_TLV_PCE); /* Set PCE Address */ pce->pce_address.header.type = htons(RI_PCE_SUBTLV_ADDRESS); - pce->pce_address.header.length = htons(PCE_ADDRESS_LENGTH_IPV4); - pce->pce_address.address.type = htons(PCE_ADDRESS_TYPE_IPV4); + pce->pce_address.header.length = htons(PCE_ADDRESS_IPV4_SIZE); + pce->pce_address.address.type = htons(PCE_ADDRESS_IPV4); pce->pce_address.address.value = ipv4; return; @@ -315,7 +315,7 @@ static void set_pce_domain(uint16_t type, uint32_t domain, sizeof(struct ri_pce_subtlv_domain)); new->header.type = htons(RI_PCE_SUBTLV_DOMAIN); - new->header.length = htons(PCE_ADDRESS_LENGTH_IPV4); + new->header.length = htons(PCE_ADDRESS_IPV4_SIZE); new->type = htons(type); new->value = htonl(domain); @@ -361,7 +361,7 @@ static void set_pce_neighbor(uint16_t type, uint32_t domain, sizeof(struct ri_pce_subtlv_neighbor)); new->header.type = htons(RI_PCE_SUBTLV_NEIGHBOR); - new->header.length = htons(PCE_ADDRESS_LENGTH_IPV4); + new->header.length = htons(PCE_ADDRESS_IPV4_SIZE); new->type = htons(type); new->value = htonl(domain); @@ -1184,10 +1184,25 @@ static int ospf_router_info_lsa_update(struct ospf_lsa *lsa) * Followings are vty session control functions. *------------------------------------------------------------------------*/ +#define check_tlv_size(size, msg) \ + do { \ + if (ntohs(tlvh->length) > size) { \ + if (vty != NULL) \ + vty_out(vty, " Wrong %s TLV size: %d(%d)\n", \ + msg, ntohs(tlvh->length), size); \ + else \ + zlog_debug(" Wrong %s TLV size: %d(%d)\n", \ + msg, ntohs(tlvh->length), size); \ + return size + TLV_HDR_SIZE; \ + } \ + } while (0) + static uint16_t show_vty_router_cap(struct vty *vty, struct tlv_header *tlvh) { struct ri_tlv_router_cap *top = (struct ri_tlv_router_cap *)tlvh; + check_tlv_size(RI_TLV_CAPABILITIES_SIZE, "Router Capabilities"); + if (vty != NULL) vty_out(vty, " Router Capabilities: 0x%x\n", ntohl(top->value)); @@ -1203,21 +1218,30 @@ static uint16_t show_vty_pce_subtlv_address(struct vty *vty, struct ri_pce_subtlv_address *top = (struct ri_pce_subtlv_address *)tlvh; - if (ntohs(top->address.type) == PCE_ADDRESS_TYPE_IPV4) { + if (ntohs(top->address.type) == PCE_ADDRESS_IPV4) { + check_tlv_size(PCE_ADDRESS_IPV4_SIZE, "PCE Address"); if (vty != NULL) vty_out(vty, " PCE Address: %s\n", inet_ntoa(top->address.value)); else zlog_debug(" PCE Address: %s", inet_ntoa(top->address.value)); - } else { + } else if (ntohs(top->address.type) == PCE_ADDRESS_IPV6) { /* TODO: Add support to IPv6 with inet_ntop() */ + check_tlv_size(PCE_ADDRESS_IPV6_SIZE, "PCE Address"); if (vty != NULL) vty_out(vty, " PCE Address: 0x%x\n", ntohl(top->address.value.s_addr)); else zlog_debug(" PCE Address: 0x%x", ntohl(top->address.value.s_addr)); + } else { + if (vty != NULL) + vty_out(vty, " Wrong PCE Address type: 0x%x\n", + ntohl(top->address.type)); + else + zlog_debug(" Wrong PCE Address type: 0x%x", + ntohl(top->address.type)); } return TLV_SIZE(tlvh); @@ -1229,6 +1253,8 @@ static uint16_t show_vty_pce_subtlv_path_scope(struct vty *vty, struct ri_pce_subtlv_path_scope *top = (struct ri_pce_subtlv_path_scope *)tlvh; + check_tlv_size(RI_PCE_SUBTLV_PATH_SCOPE_SIZE, "PCE Path Scope"); + if (vty != NULL) vty_out(vty, " PCE Path Scope: 0x%x\n", ntohl(top->value)); else @@ -1243,18 +1269,27 @@ static uint16_t show_vty_pce_subtlv_domain(struct vty *vty, struct ri_pce_subtlv_domain *top = (struct ri_pce_subtlv_domain *)tlvh; struct in_addr tmp; + check_tlv_size(RI_PCE_SUBTLV_DOMAIN_SIZE, "PCE Domain"); + if (ntohs(top->type) == PCE_DOMAIN_TYPE_AREA) { tmp.s_addr = top->value; if (vty != NULL) - vty_out(vty, " PCE domain Area: %s\n", inet_ntoa(tmp)); + vty_out(vty, " PCE Domain Area: %pI4\n", &tmp); else - zlog_debug(" PCE domain Area: %s", inet_ntoa(tmp)); - } else { + zlog_debug(" PCE Domain Area: %pI4", &tmp); + } else if (ntohs(top->type) == PCE_DOMAIN_TYPE_AS) { if (vty != NULL) - vty_out(vty, " PCE domain AS: %d\n", + vty_out(vty, " PCE Domain AS: %d\n", ntohl(top->value)); else - zlog_debug(" PCE domain AS: %d", ntohl(top->value)); + zlog_debug(" PCE Domain AS: %d", ntohl(top->value)); + } else { + if (vty != NULL) + vty_out(vty, " Wrong PCE Domain type: %d\n", + ntohl(top->type)); + else + zlog_debug(" Wrong PCE Domain type: %d", + ntohl(top->type)); } return TLV_SIZE(tlvh); } @@ -1267,20 +1302,28 @@ static uint16_t show_vty_pce_subtlv_neighbor(struct vty *vty, (struct ri_pce_subtlv_neighbor *)tlvh; struct in_addr tmp; + check_tlv_size(RI_PCE_SUBTLV_NEIGHBOR_SIZE, "PCE Neighbor"); + if (ntohs(top->type) == PCE_DOMAIN_TYPE_AREA) { tmp.s_addr = top->value; if (vty != NULL) - vty_out(vty, " PCE neighbor Area: %s\n", - inet_ntoa(tmp)); + vty_out(vty, " PCE Neighbor Area: %pI4\n", &tmp); else - zlog_debug(" PCE neighbor Area: %s", inet_ntoa(tmp)); - } else { + zlog_debug(" PCE Neighbor Area: %pI4\n", &tmp); + } else if (ntohs(top->type) == PCE_DOMAIN_TYPE_AS) { if (vty != NULL) - vty_out(vty, " PCE neighbor AS: %d\n", + vty_out(vty, " PCE Neighbor AS: %d\n", ntohl(top->value)); else - zlog_debug(" PCE neighbor AS: %d", + zlog_debug(" PCE Neighbor AS: %d", ntohl(top->value)); + } else { + if (vty != NULL) + vty_out(vty, " Wrong PCE Neighbor type: %d\n", + ntohl(top->type)); + else + zlog_debug(" Wrong PCE Neighbor type: %d", + ntohl(top->type)); } return TLV_SIZE(tlvh); } @@ -1291,6 +1334,8 @@ static uint16_t show_vty_pce_subtlv_cap_flag(struct vty *vty, struct ri_pce_subtlv_cap_flag *top = (struct ri_pce_subtlv_cap_flag *)tlvh; + check_tlv_size(RI_PCE_SUBTLV_CAP_FLAG_SIZE, "PCE Capabilities"); + if (vty != NULL) vty_out(vty, " PCE Capabilities Flag: 0x%x\n", ntohl(top->value)); @@ -1301,8 +1346,21 @@ static uint16_t show_vty_pce_subtlv_cap_flag(struct vty *vty, return TLV_SIZE(tlvh); } -static uint16_t show_vty_unknown_tlv(struct vty *vty, struct tlv_header *tlvh) +static uint16_t show_vty_unknown_tlv(struct vty *vty, struct tlv_header *tlvh, + size_t buf_size) { + if (TLV_SIZE(tlvh) > buf_size) { + if (vty != NULL) + vty_out(vty, + " TLV size %d exceeds buffer size. Abort!", + TLV_SIZE(tlvh)); + else + zlog_debug( + " TLV size %d exceeds buffer size. Abort!", + TLV_SIZE(tlvh)); + return buf_size; + } + if (vty != NULL) vty_out(vty, " Unknown TLV: [type(0x%x), length(0x%x)]\n", ntohs(tlvh->type), ntohs(tlvh->length)); @@ -1314,12 +1372,26 @@ static uint16_t show_vty_unknown_tlv(struct vty *vty, struct tlv_header *tlvh) } static uint16_t show_vty_pce_info(struct vty *vty, struct tlv_header *ri, - uint32_t total) + size_t buf_size) { struct tlv_header *tlvh; + uint16_t length = ntohs(ri->length); uint16_t sum = 0; - for (tlvh = ri; sum < total; tlvh = TLV_HDR_NEXT(tlvh)) { + /* Verify that TLV length is valid against remaining buffer size */ + if (length > buf_size) { + if (vty != NULL) + vty_out(vty, + " PCE Info TLV size %d exceeds buffer size. Abort!\n", + length); + else + zlog_debug( + " PCE Info TLV size %d exceeds buffer size. Abort!\n", + length); + return buf_size; + } + + for (tlvh = ri; sum < length; tlvh = TLV_HDR_NEXT(tlvh)) { switch (ntohs(tlvh->type)) { case RI_PCE_SUBTLV_ADDRESS: sum += show_vty_pce_subtlv_address(vty, tlvh); @@ -1337,7 +1409,7 @@ static uint16_t show_vty_pce_info(struct vty *vty, struct tlv_header *ri, sum += show_vty_pce_subtlv_cap_flag(vty, tlvh); break; default: - sum += show_vty_unknown_tlv(vty, tlvh); + sum += show_vty_unknown_tlv(vty, tlvh, length - sum); break; } } @@ -1351,6 +1423,8 @@ static uint16_t show_vty_sr_algorithm(struct vty *vty, struct tlv_header *tlvh) (struct ri_sr_tlv_sr_algorithm *)tlvh; int i; + check_tlv_size(ALGORITHM_COUNT, "Segment Routing Algorithm"); + if (vty != NULL) { vty_out(vty, " Segment Routing Algorithm TLV:\n"); for (i = 0; i < ntohs(algo->header.length); i++) { @@ -1369,9 +1443,7 @@ static uint16_t show_vty_sr_algorithm(struct vty *vty, struct tlv_header *tlvh) break; } } - } - - else { + } else { zlog_debug(" Segment Routing Algorithm TLV:"); for (i = 0; i < ntohs(algo->header.length); i++) switch (algo->value[i]) { @@ -1397,6 +1469,8 @@ static uint16_t show_vty_sr_range(struct vty *vty, struct tlv_header *tlvh) struct ri_sr_tlv_sid_label_range *range = (struct ri_sr_tlv_sid_label_range *)tlvh; + check_tlv_size(RI_SR_TLV_LABEL_RANGE_SIZE, "SR Label Range"); + if (vty != NULL) { vty_out(vty, " Segment Routing Range TLV:\n" @@ -1421,6 +1495,8 @@ static uint16_t show_vty_sr_msd(struct vty *vty, struct tlv_header *tlvh) { struct ri_sr_tlv_node_msd *msd = (struct ri_sr_tlv_node_msd *)tlvh; + check_tlv_size(RI_SR_TLV_NODE_MSD_SIZE, "Node Maximum Stack Depth"); + if (vty != NULL) { vty_out(vty, " Segment Routing MSD TLV:\n" @@ -1443,9 +1519,9 @@ static void ospf_router_info_show_info(struct vty *vty, struct ospf_lsa *lsa) uint16_t length = 0, sum = 0; /* Initialize TLV browsing */ - length = ntohs(lsah->length) - OSPF_LSA_HEADER_SIZE; + length = lsa->size - OSPF_LSA_HEADER_SIZE; - for (tlvh = TLV_HDR_TOP(lsah); sum < length; + for (tlvh = TLV_HDR_TOP(lsah); sum < length && tlvh; tlvh = TLV_HDR_NEXT(tlvh)) { switch (ntohs(tlvh->type)) { case RI_TLV_CAPABILITIES: @@ -1467,7 +1543,7 @@ static void ospf_router_info_show_info(struct vty *vty, struct ospf_lsa *lsa) break; default: - sum += show_vty_unknown_tlv(vty, tlvh); + sum += show_vty_unknown_tlv(vty, tlvh, length); break; } } diff --git a/ospfd/ospf_ri.h b/ospfd/ospf_ri.h index 84511ac5e7..f5650444fc 100644 --- a/ospfd/ospf_ri.h +++ b/ospfd/ospf_ri.h @@ -75,7 +75,7 @@ /* RFC4970: Router Information Capabilities TLV */ /* Mandatory */ #define RI_TLV_CAPABILITIES 1 - +#define RI_TLV_CAPABILITIES_SIZE 4 struct ri_tlv_router_cap { struct tlv_header header; /* Value length is 4 bytes. */ uint32_t value; @@ -105,12 +105,12 @@ struct ri_tlv_pce { struct ri_pce_subtlv_address { /* Type = 1; Length is 8 (IPv4) or 20 (IPv6) bytes. */ struct tlv_header header; -#define PCE_ADDRESS_LENGTH_IPV4 8 -#define PCE_ADDRESS_LENGTH_IPV6 20 +#define PCE_ADDRESS_IPV4_SIZE 8 +#define PCE_ADDRESS_IPV6_SIZE 20 struct { uint16_t type; /* Address type: 1 = IPv4, 2 = IPv6 */ -#define PCE_ADDRESS_TYPE_IPV4 1 -#define PCE_ADDRESS_TYPE_IPV6 2 +#define PCE_ADDRESS_IPV4 1 +#define PCE_ADDRESS_IPV6 2 uint16_t reserved; struct in_addr value; /* PCE address */ } address; @@ -118,6 +118,7 @@ struct ri_pce_subtlv_address { /* PCE Path-Scope Sub-TLV */ /* Mandatory */ #define RI_PCE_SUBTLV_PATH_SCOPE 2 +#define RI_PCE_SUBTLV_PATH_SCOPE_SIZE 4 struct ri_pce_subtlv_path_scope { struct tlv_header header; /* Type = 2; Length = 4 bytes. */ /* @@ -128,11 +129,11 @@ struct ri_pce_subtlv_path_scope { }; /* PCE Domain Sub-TLV */ /* Optional */ -#define RI_PCE_SUBTLV_DOMAIN 3 - #define PCE_DOMAIN_TYPE_AREA 1 -#define PCE_DOMAIN_TYPE_AS 2 +#define PCE_DOMAIN_TYPE_AS 2 +#define RI_PCE_SUBTLV_DOMAIN 3 +#define RI_PCE_SUBTLV_DOMAIN_SIZE 8 struct ri_pce_subtlv_domain { struct tlv_header header; /* Type = 3; Length = 8 bytes. */ uint16_t type; /* Domain type: 1 = OSPF Area ID, 2 = AS Number */ @@ -142,6 +143,7 @@ struct ri_pce_subtlv_domain { /* PCE Neighbor Sub-TLV */ /* Mandatory if R or S bit is set */ #define RI_PCE_SUBTLV_NEIGHBOR 4 +#define RI_PCE_SUBTLV_NEIGHBOR_SIZE 8 struct ri_pce_subtlv_neighbor { struct tlv_header header; /* Type = 4; Length = 8 bytes. */ uint16_t type; /* Domain type: 1 = OSPF Area ID, 2 = AS Number */ @@ -151,6 +153,7 @@ struct ri_pce_subtlv_neighbor { /* PCE Capabilities Flags Sub-TLV */ /* Optional */ #define RI_PCE_SUBTLV_CAP_FLAG 5 +#define RI_PCE_SUBTLV_CAP_FLAG_SIZE 4 #define PCE_CAP_GMPLS_LINK 0x0001 #define PCE_CAP_BIDIRECTIONAL 0x0002 diff --git a/ospfd/ospf_sr.c b/ospfd/ospf_sr.c index 2a35bd2ef9..80230791ac 100644 --- a/ospfd/ospf_sr.c +++ b/ospfd/ospf_sr.c @@ -667,7 +667,7 @@ static inline void update_sid_nhlfe(struct sr_nhlfe n1, struct sr_nhlfe n2) */ /* Extended Link SubTLVs Getter */ -static struct sr_link *get_ext_link_sid(struct tlv_header *tlvh) +static struct sr_link *get_ext_link_sid(struct tlv_header *tlvh, size_t size) { struct sr_link *srl; @@ -679,13 +679,20 @@ static struct sr_link *get_ext_link_sid(struct tlv_header *tlvh) struct tlv_header *sub_tlvh; uint16_t length = 0, sum = 0, i = 0; + /* Check TLV size */ + if ((ntohs(tlvh->length) > size) + || ntohs(tlvh->length) < EXT_TLV_LINK_SIZE) { + zlog_warn("Wrong Extended Link TLV size. Abort!"); + return NULL; + } + srl = XCALLOC(MTYPE_OSPF_SR_PARAMS, sizeof(struct sr_link)); /* Initialize TLV browsing */ length = ntohs(tlvh->length) - EXT_TLV_LINK_SIZE; sub_tlvh = (struct tlv_header *)((char *)(tlvh) + TLV_HDR_SIZE + EXT_TLV_LINK_SIZE); - for (; sum < length; sub_tlvh = TLV_HDR_NEXT(sub_tlvh)) { + for (; sum < length && sub_tlvh; sub_tlvh = TLV_HDR_NEXT(sub_tlvh)) { switch (ntohs(sub_tlvh->type)) { case EXT_SUBTLV_ADJ_SID: adj_sid = (struct ext_subtlv_adj_sid *)sub_tlvh; @@ -751,7 +758,8 @@ static struct sr_link *get_ext_link_sid(struct tlv_header *tlvh) } /* Extended Prefix SubTLVs Getter */ -static struct sr_prefix *get_ext_prefix_sid(struct tlv_header *tlvh) +static struct sr_prefix *get_ext_prefix_sid(struct tlv_header *tlvh, + size_t size) { struct sr_prefix *srp; @@ -761,13 +769,20 @@ static struct sr_prefix *get_ext_prefix_sid(struct tlv_header *tlvh) struct tlv_header *sub_tlvh; uint16_t length = 0, sum = 0; + /* Check TLV size */ + if ((ntohs(tlvh->length) > size) + || ntohs(tlvh->length) < EXT_TLV_PREFIX_SIZE) { + zlog_warn("Wrong Extended Link TLV size. Abort!"); + return NULL; + } + srp = XCALLOC(MTYPE_OSPF_SR_PARAMS, sizeof(struct sr_prefix)); /* Initialize TLV browsing */ length = ntohs(tlvh->length) - EXT_TLV_PREFIX_SIZE; sub_tlvh = (struct tlv_header *)((char *)(tlvh) + TLV_HDR_SIZE + EXT_TLV_PREFIX_SIZE); - for (; sum < length; sub_tlvh = TLV_HDR_NEXT(sub_tlvh)) { + for (; sum < length && sub_tlvh; sub_tlvh = TLV_HDR_NEXT(sub_tlvh)) { switch (ntohs(sub_tlvh->type)) { case EXT_SUBTLV_PREFIX_SID: psid = (struct ext_subtlv_prefix_sid *)sub_tlvh; @@ -1081,7 +1096,7 @@ void ospf_sr_ri_lsa_update(struct ospf_lsa *lsa) /* Collect Router Information Sub TLVs */ /* Initialize TLV browsing */ - length = ntohs(lsah->length) - OSPF_LSA_HEADER_SIZE; + length = lsa->size - OSPF_LSA_HEADER_SIZE; srgb.range_size = 0; srgb.lower_bound = 0; @@ -1096,23 +1111,20 @@ void ospf_sr_ri_lsa_update(struct ospf_lsa *lsa) srn->algo[i] = algo->value[0]; for (; i < ALGORITHM_COUNT; i++) srn->algo[i] = SR_ALGORITHM_UNSET; - sum += TLV_SIZE(tlvh); break; case RI_SR_TLV_SID_LABEL_RANGE: ri_srgb = (struct ri_sr_tlv_sid_label_range *)tlvh; srgb.range_size = GET_RANGE_SIZE(ntohl(ri_srgb->size)); srgb.lower_bound = GET_LABEL(ntohl(ri_srgb->lower.value)); - sum += TLV_SIZE(tlvh); break; case RI_SR_TLV_NODE_MSD: srn->msd = ((struct ri_sr_tlv_node_msd *)(tlvh))->value; - sum += TLV_SIZE(tlvh); break; default: - sum += TLV_SIZE(tlvh); break; } + sum += TLV_SIZE(tlvh); } /* Check that we collect mandatory parameters */ @@ -1201,7 +1213,7 @@ void ospf_sr_ext_link_lsa_update(struct ospf_lsa *lsa) struct lsa_header *lsah = lsa->data; struct sr_link *srl; - uint16_t length, sum; + int length; if (IS_DEBUG_OSPF_SR) zlog_debug( @@ -1230,20 +1242,19 @@ void ospf_sr_ext_link_lsa_update(struct ospf_lsa *lsa) } /* Initialize TLV browsing */ - length = ntohs(lsah->length) - OSPF_LSA_HEADER_SIZE; - sum = 0; - for (tlvh = TLV_HDR_TOP(lsah); (sum < length) && (tlvh != NULL); + length = lsa->size - OSPF_LSA_HEADER_SIZE; + for (tlvh = TLV_HDR_TOP(lsah); length > 0 && tlvh; tlvh = TLV_HDR_NEXT(tlvh)) { if (ntohs(tlvh->type) == EXT_TLV_LINK) { /* Got Extended Link information */ - srl = get_ext_link_sid(tlvh); + srl = get_ext_link_sid(tlvh, length); /* Update SID if not null */ if (srl != NULL) { srl->instance = ntohl(lsah->id.s_addr); update_ext_link_sid(srn, srl, lsa->flags); } } - sum += TLV_SIZE(tlvh); + length -= TLV_SIZE(tlvh); } } @@ -1311,7 +1322,7 @@ void ospf_sr_ext_prefix_lsa_update(struct ospf_lsa *lsa) struct lsa_header *lsah = lsa->data; struct sr_prefix *srp; - uint16_t length, sum; + int length; if (IS_DEBUG_OSPF_SR) zlog_debug( @@ -1341,20 +1352,19 @@ void ospf_sr_ext_prefix_lsa_update(struct ospf_lsa *lsa) } /* Initialize TLV browsing */ - length = ntohs(lsah->length) - OSPF_LSA_HEADER_SIZE; - sum = 0; - for (tlvh = TLV_HDR_TOP(lsah); sum < length; + length = lsa->size - OSPF_LSA_HEADER_SIZE; + for (tlvh = TLV_HDR_TOP(lsah); length > 0 && tlvh; tlvh = TLV_HDR_NEXT(tlvh)) { if (ntohs(tlvh->type) == EXT_TLV_LINK) { /* Got Extended Link information */ - srp = get_ext_prefix_sid(tlvh); + srp = get_ext_prefix_sid(tlvh, length); /* Update SID if not null */ if (srp != NULL) { srp->instance = ntohl(lsah->id.s_addr); update_ext_prefix_sid(srn, srp); } } - sum += TLV_SIZE(tlvh); + length -= TLV_SIZE(tlvh); } } diff --git a/ospfd/ospf_sr.h b/ospfd/ospf_sr.h index df923e970f..3dcff1512a 100644 --- a/ospfd/ospf_sr.h +++ b/ospfd/ospf_sr.h @@ -79,6 +79,8 @@ struct ri_sr_tlv_sr_algorithm { /* RI SID/Label Range TLV - section 3.2 */ #define RI_SR_TLV_SID_LABEL_RANGE 9 +#define RI_SR_TLV_SRLB_LABEL_RANGE 14 +#define RI_SR_TLV_LABEL_RANGE_SIZE 12 struct ri_sr_tlv_sid_label_range { struct tlv_header header; /* Only 24 upper most bits are significant */ @@ -90,6 +92,7 @@ struct ri_sr_tlv_sid_label_range { /* RI Node/MSD TLV as per draft-ietf-ospf-segment-routing-msd-05 */ #define RI_SR_TLV_NODE_MSD 12 +#define RI_SR_TLV_NODE_MSD_SIZE 4 struct ri_sr_tlv_node_msd { struct tlv_header header; uint8_t subtype; /* always = 1 */ diff --git a/ospfd/ospf_te.c b/ospfd/ospf_te.c index 1009c7577e..b078ac7400 100644 --- a/ospfd/ospf_te.c +++ b/ospfd/ospf_te.c @@ -1587,11 +1587,25 @@ void ospf_mpls_te_lsa_schedule(struct mpls_te_link *lp, enum lsa_opcode opcode) /*------------------------------------------------------------------------* * Followings are vty session control functions. *------------------------------------------------------------------------*/ +#define check_tlv_size(size, msg) \ + do { \ + if (ntohs(tlvh->length) > size) { \ + if (vty != NULL) \ + vty_out(vty, " Wrong %s TLV size: %d(%d)\n", \ + msg, ntohs(tlvh->length), size); \ + else \ + zlog_debug(" Wrong %s TLV size: %d(%d)\n", \ + msg, ntohs(tlvh->length), size); \ + return size + TLV_HDR_SIZE; \ + } \ + } while(0) static uint16_t show_vty_router_addr(struct vty *vty, struct tlv_header *tlvh) { struct te_tlv_router_addr *top = (struct te_tlv_router_addr *)tlvh; + check_tlv_size(TE_LINK_SUBTLV_DEF_SIZE, "Router Address"); + if (vty != NULL) vty_out(vty, " Router-Address: %s\n", inet_ntoa(top->value)); else @@ -1600,10 +1614,23 @@ static uint16_t show_vty_router_addr(struct vty *vty, struct tlv_header *tlvh) return TLV_SIZE(tlvh); } -static uint16_t show_vty_link_header(struct vty *vty, struct tlv_header *tlvh) +static uint16_t show_vty_link_header(struct vty *vty, struct tlv_header *tlvh, + size_t buf_size) { struct te_tlv_link *top = (struct te_tlv_link *)tlvh; + if (TLV_SIZE(tlvh) > buf_size) { + if (vty != NULL) + vty_out(vty, + " TLV size %d exceeds buffer size. Abort!", + TLV_SIZE(tlvh)); + else + zlog_debug( + " TLV size %d exceeds buffer size. Abort!", + TLV_SIZE(tlvh)); + return buf_size; + } + if (vty != NULL) vty_out(vty, " Link: %u octets of data\n", ntohs(top->header.length)); @@ -1620,6 +1647,8 @@ static uint16_t show_vty_link_subtlv_link_type(struct vty *vty, struct te_link_subtlv_link_type *top; const char *cp = "Unknown"; + check_tlv_size(TE_LINK_SUBTLV_TYPE_SIZE, "Link Type"); + top = (struct te_link_subtlv_link_type *)tlvh; switch (top->link_type.value) { case LINK_TYPE_SUBTLV_VALUE_PTP: @@ -1646,6 +1675,8 @@ static uint16_t show_vty_link_subtlv_link_id(struct vty *vty, { struct te_link_subtlv_link_id *top; + check_tlv_size(TE_LINK_SUBTLV_DEF_SIZE, "Link ID"); + top = (struct te_link_subtlv_link_id *)tlvh; if (vty != NULL) vty_out(vty, " Link-ID: %s\n", inet_ntoa(top->value)); @@ -1656,11 +1687,24 @@ static uint16_t show_vty_link_subtlv_link_id(struct vty *vty, } static uint16_t show_vty_link_subtlv_lclif_ipaddr(struct vty *vty, - struct tlv_header *tlvh) + struct tlv_header *tlvh, + size_t buf_size) { struct te_link_subtlv_lclif_ipaddr *top; int i, n; + if (TLV_SIZE(tlvh) > buf_size) { + if (vty != NULL) + vty_out(vty, + " TLV size %d exceeds buffer size. Abort!", + TLV_SIZE(tlvh)); + else + zlog_debug( + " TLV size %d exceeds buffer size. Abort!", + TLV_SIZE(tlvh)); + return buf_size; + } + top = (struct te_link_subtlv_lclif_ipaddr *)tlvh; n = ntohs(tlvh->length) / sizeof(top->value[0]); @@ -1681,11 +1725,24 @@ static uint16_t show_vty_link_subtlv_lclif_ipaddr(struct vty *vty, } static uint16_t show_vty_link_subtlv_rmtif_ipaddr(struct vty *vty, - struct tlv_header *tlvh) + struct tlv_header *tlvh, + size_t buf_size) { struct te_link_subtlv_rmtif_ipaddr *top; int i, n; + if (TLV_SIZE(tlvh) > buf_size) { + if (vty != NULL) + vty_out(vty, + " TLV size %d exceeds buffer size. Abort!", + TLV_SIZE(tlvh)); + else + zlog_debug( + " TLV size %d exceeds buffer size. Abort!", + TLV_SIZE(tlvh)); + return buf_size; + } + top = (struct te_link_subtlv_rmtif_ipaddr *)tlvh; n = ntohs(tlvh->length) / sizeof(top->value[0]); if (vty != NULL) @@ -1709,6 +1766,8 @@ static uint16_t show_vty_link_subtlv_te_metric(struct vty *vty, { struct te_link_subtlv_te_metric *top; + check_tlv_size(TE_LINK_SUBTLV_DEF_SIZE, "TE Metric"); + top = (struct te_link_subtlv_te_metric *)tlvh; if (vty != NULL) vty_out(vty, " Traffic Engineering Metric: %u\n", @@ -1726,6 +1785,8 @@ static uint16_t show_vty_link_subtlv_max_bw(struct vty *vty, struct te_link_subtlv_max_bw *top; float fval; + check_tlv_size(TE_LINK_SUBTLV_DEF_SIZE, "Maximum Bandwidth"); + top = (struct te_link_subtlv_max_bw *)tlvh; fval = ntohf(top->value); @@ -1743,6 +1804,8 @@ static uint16_t show_vty_link_subtlv_max_rsv_bw(struct vty *vty, struct te_link_subtlv_max_rsv_bw *top; float fval; + check_tlv_size(TE_LINK_SUBTLV_DEF_SIZE, "Maximum Reservable Bandwidth"); + top = (struct te_link_subtlv_max_rsv_bw *)tlvh; fval = ntohf(top->value); @@ -1763,6 +1826,8 @@ static uint16_t show_vty_link_subtlv_unrsv_bw(struct vty *vty, float fval1, fval2; int i; + check_tlv_size(TE_LINK_SUBTLV_UNRSV_SIZE, "Unreserved Bandwidth"); + top = (struct te_link_subtlv_unrsv_bw *)tlvh; if (vty != NULL) vty_out(vty, @@ -1792,6 +1857,8 @@ static uint16_t show_vty_link_subtlv_rsc_clsclr(struct vty *vty, { struct te_link_subtlv_rsc_clsclr *top; + check_tlv_size(TE_LINK_SUBTLV_DEF_SIZE, "Resource class/color"); + top = (struct te_link_subtlv_rsc_clsclr *)tlvh; if (vty != NULL) vty_out(vty, " Resource class/color: 0x%x\n", @@ -1808,6 +1875,8 @@ static uint16_t show_vty_link_subtlv_lrrid(struct vty *vty, { struct te_link_subtlv_lrrid *top; + check_tlv_size(TE_LINK_SUBTLV_LRRID_SIZE, "Local/Remote Router ID"); + top = (struct te_link_subtlv_lrrid *)tlvh; if (vty != NULL) { @@ -1830,6 +1899,8 @@ static uint16_t show_vty_link_subtlv_llri(struct vty *vty, { struct te_link_subtlv_llri *top; + check_tlv_size(TE_LINK_SUBTLV_LLRI_SIZE, "Link Local/Remote ID"); + top = (struct te_link_subtlv_llri *)tlvh; if (vty != NULL) { @@ -1852,6 +1923,8 @@ static uint16_t show_vty_link_subtlv_rip(struct vty *vty, { struct te_link_subtlv_rip *top; + check_tlv_size(TE_LINK_SUBTLV_DEF_SIZE, "Remote ASBR Address"); + top = (struct te_link_subtlv_rip *)tlvh; if (vty != NULL) @@ -1869,6 +1942,8 @@ static uint16_t show_vty_link_subtlv_ras(struct vty *vty, { struct te_link_subtlv_ras *top; + check_tlv_size(TE_LINK_SUBTLV_DEF_SIZE, "Remote AS number"); + top = (struct te_link_subtlv_ras *)tlvh; if (vty != NULL) @@ -1888,6 +1963,8 @@ static uint16_t show_vty_link_subtlv_av_delay(struct vty *vty, uint32_t delay; uint32_t anomalous; + check_tlv_size(TE_LINK_SUBTLV_DEF_SIZE, "Average Link Delay"); + top = (struct te_link_subtlv_av_delay *)tlvh; delay = (uint32_t)ntohl(top->value) & TE_EXT_MASK; anomalous = (uint32_t)ntohl(top->value) & TE_EXT_ANORMAL; @@ -1909,6 +1986,8 @@ static uint16_t show_vty_link_subtlv_mm_delay(struct vty *vty, uint32_t low, high; uint32_t anomalous; + check_tlv_size(TE_LINK_SUBTLV_MM_DELAY_SIZE, "Min/Max Link Delay"); + top = (struct te_link_subtlv_mm_delay *)tlvh; low = (uint32_t)ntohl(top->low) & TE_EXT_MASK; anomalous = (uint32_t)ntohl(top->low) & TE_EXT_ANORMAL; @@ -1930,6 +2009,8 @@ static uint16_t show_vty_link_subtlv_delay_var(struct vty *vty, struct te_link_subtlv_delay_var *top; uint32_t jitter; + check_tlv_size(TE_LINK_SUBTLV_DEF_SIZE, "Link Delay Variation"); + top = (struct te_link_subtlv_delay_var *)tlvh; jitter = (uint32_t)ntohl(top->value) & TE_EXT_MASK; @@ -1949,6 +2030,8 @@ static uint16_t show_vty_link_subtlv_pkt_loss(struct vty *vty, uint32_t anomalous; float fval; + check_tlv_size(TE_LINK_SUBTLV_DEF_SIZE, "Link Loss"); + top = (struct te_link_subtlv_pkt_loss *)tlvh; loss = (uint32_t)ntohl(top->value) & TE_EXT_MASK; fval = (float)(loss * LOSS_PRECISION); @@ -1970,6 +2053,8 @@ static uint16_t show_vty_link_subtlv_res_bw(struct vty *vty, struct te_link_subtlv_res_bw *top; float fval; + check_tlv_size(TE_LINK_SUBTLV_DEF_SIZE, "Residual Bandwidth"); + top = (struct te_link_subtlv_res_bw *)tlvh; fval = ntohf(top->value); @@ -1991,6 +2076,8 @@ static uint16_t show_vty_link_subtlv_ava_bw(struct vty *vty, struct te_link_subtlv_ava_bw *top; float fval; + check_tlv_size(TE_LINK_SUBTLV_DEF_SIZE, "Available Bandwidth"); + top = (struct te_link_subtlv_ava_bw *)tlvh; fval = ntohf(top->value); @@ -2012,6 +2099,8 @@ static uint16_t show_vty_link_subtlv_use_bw(struct vty *vty, struct te_link_subtlv_use_bw *top; float fval; + check_tlv_size(TE_LINK_SUBTLV_DEF_SIZE, "Utilized Bandwidth"); + top = (struct te_link_subtlv_use_bw *)tlvh; fval = ntohf(top->value); @@ -2027,8 +2116,21 @@ static uint16_t show_vty_link_subtlv_use_bw(struct vty *vty, return TLV_SIZE(tlvh); } -static uint16_t show_vty_unknown_tlv(struct vty *vty, struct tlv_header *tlvh) +static uint16_t show_vty_unknown_tlv(struct vty *vty, struct tlv_header *tlvh, + size_t buf_size) { + if (TLV_SIZE(tlvh) > buf_size) { + if (vty != NULL) + vty_out(vty, + " TLV size %d exceeds buffer size. Abort!", + TLV_SIZE(tlvh)); + else + zlog_debug( + " TLV size %d exceeds buffer size. Abort!", + TLV_SIZE(tlvh)); + return buf_size; + } + if (vty != NULL) vty_out(vty, " Unknown TLV: [type(0x%x), length(0x%x)]\n", ntohs(tlvh->type), ntohs(tlvh->length)); @@ -2046,7 +2148,7 @@ static uint16_t ospf_mpls_te_show_link_subtlv(struct vty *vty, struct tlv_header *tlvh; uint16_t sum = subtotal; - for (tlvh = tlvh0; sum < total; + for (tlvh = tlvh0; sum < total && tlvh; tlvh = TLV_HDR_NEXT(tlvh)) { switch (ntohs(tlvh->type)) { case TE_LINK_SUBTLV_LINK_TYPE: @@ -2056,10 +2158,12 @@ static uint16_t ospf_mpls_te_show_link_subtlv(struct vty *vty, sum += show_vty_link_subtlv_link_id(vty, tlvh); break; case TE_LINK_SUBTLV_LCLIF_IPADDR: - sum += show_vty_link_subtlv_lclif_ipaddr(vty, tlvh); + sum += show_vty_link_subtlv_lclif_ipaddr(vty, tlvh, + total - sum); break; case TE_LINK_SUBTLV_RMTIF_IPADDR: - sum += show_vty_link_subtlv_rmtif_ipaddr(vty, tlvh); + sum += show_vty_link_subtlv_rmtif_ipaddr(vty, tlvh, + total - sum); break; case TE_LINK_SUBTLV_TE_METRIC: sum += show_vty_link_subtlv_te_metric(vty, tlvh); @@ -2110,7 +2214,7 @@ static uint16_t ospf_mpls_te_show_link_subtlv(struct vty *vty, sum += show_vty_link_subtlv_use_bw(vty, tlvh); break; default: - sum += show_vty_unknown_tlv(vty, tlvh); + sum += show_vty_unknown_tlv(vty, tlvh, total - sum); break; } } @@ -2126,9 +2230,9 @@ static void ospf_mpls_te_show_info(struct vty *vty, struct ospf_lsa *lsa) uint16_t subtotal, uint16_t total) = NULL; sum = 0; - total = ntohs(lsah->length) - OSPF_LSA_HEADER_SIZE; + total = lsa->size - OSPF_LSA_HEADER_SIZE; - for (tlvh = TLV_HDR_TOP(lsah); sum < total; + for (tlvh = TLV_HDR_TOP(lsah); sum < total && tlvh; tlvh = (next ? next : TLV_HDR_NEXT(tlvh))) { if (subfunc != NULL) { sum = (*subfunc)(vty, tlvh, sum, total); @@ -2143,12 +2247,12 @@ static void ospf_mpls_te_show_info(struct vty *vty, struct ospf_lsa *lsa) sum += show_vty_router_addr(vty, tlvh); break; case TE_TLV_LINK: - sum += show_vty_link_header(vty, tlvh); + sum += show_vty_link_header(vty, tlvh, total - sum); subfunc = ospf_mpls_te_show_link_subtlv; next = TLV_DATA(tlvh); break; default: - sum += show_vty_unknown_tlv(vty, tlvh); + sum += show_vty_unknown_tlv(vty, tlvh, total - sum); break; } } @@ -2469,10 +2573,12 @@ static void show_mpls_te_link_sub(struct vty *vty, struct interface *ifp) show_vty_link_subtlv_link_id(vty, &lp->link_id.header); if (TLV_TYPE(lp->lclif_ipaddr) != 0) show_vty_link_subtlv_lclif_ipaddr( - vty, &lp->lclif_ipaddr.header); + vty, &lp->lclif_ipaddr.header, + lp->lclif_ipaddr.header.length); if (TLV_TYPE(lp->rmtif_ipaddr) != 0) show_vty_link_subtlv_rmtif_ipaddr( - vty, &lp->rmtif_ipaddr.header); + vty, &lp->rmtif_ipaddr.header, + lp->rmtif_ipaddr.header.length); if (TLV_TYPE(lp->rip) != 0) show_vty_link_subtlv_rip(vty, &lp->rip.header); if (TLV_TYPE(lp->ras) != 0) diff --git a/ospfd/ospf_vty.c b/ospfd/ospf_vty.c index 71275e49d2..65ba591581 100644 --- a/ospfd/ospf_vty.c +++ b/ospfd/ospf_vty.c @@ -5916,16 +5916,18 @@ static int show_network_lsa_detail(struct vty *vty, struct ospf_lsa *lsa) if (lsa != NULL) { struct network_lsa *nl = (struct network_lsa *)lsa->data; + struct in_addr *addr; show_ip_ospf_database_header(vty, lsa); vty_out(vty, " Network Mask: /%d\n", ip_masklen(nl->mask)); - length = ntohs(lsa->data->length) - OSPF_LSA_HEADER_SIZE - 4; - - for (i = 0; length > 0; i++, length -= 4) - vty_out(vty, " Attached Router: %s\n", - inet_ntoa(nl->routers[i])); + length = lsa->size - OSPF_LSA_HEADER_SIZE - 4; + addr = &nl->routers[0]; + for (i = 0; length > 0 && addr; + length -= 4, addr = &nl->routers[++i]) + vty_out(vty, " Attached Router: %pI4\n", + addr); vty_out(vty, "\n"); } -- 2.51.0 From 902e5adbc6e133917c35511a423206fc51c9ac90 Mon Sep 17 00:00:00 2001 From: Marius Tomaschewski Date: Fri, 13 Mar 2026 11:09:43 +0100 Subject: [PATCH 2/5] ospfd: Backport SID size check Upstream: yes References: CVE-2025-61099,CVE-2025-61100,CVE-2025-61101,CVE-2025-61102,CVE-2025-61103,CVE-2025-61104,CVE-2025-61105,CVE-2025-61106,CVE-2025-61107,bsc#1252838,bsc#1252829,bsc#1252833,bsc#1252835,bsc#1252810,bsc#1252811,bsc#1252761,bsc#1252812,bsc#1252813 Backport from commit 397dbbcae6083a00f30ad8310160650ec7ac7a87 in https://github.com/FRRouting/frr/pull/14962 ``` ospfd: Correct SID check size Segment Router Identifier (SID) could be an index (4 bytes) within a range (SRGB or SRLB) or an MPLS label (3 bytes). Thus, before calling check_size macro to verify SID TLVs size, it is mandatory to determine the SID type to avoid wrong assert. Signed-off-by: Olivier Dugeon ``` Signed-off-by: Marius Tomaschewski diff --git a/ospfd/ospf_ext.c b/ospfd/ospf_ext.c index ff14087ff7..9e2ec89531 100644 --- a/ospfd/ospf_ext.c +++ b/ospfd/ospf_ext.c @@ -1610,8 +1610,12 @@ static uint16_t show_vty_ext_link_adj_sid(struct vty *vty, struct tlv_header *tlvh) { struct ext_subtlv_adj_sid *top = (struct ext_subtlv_adj_sid *)tlvh; + uint8_t tlv_size; - check_tlv_size(EXT_SUBTLV_ADJ_SID_SIZE, "Adjacency SID"); + tlv_size = CHECK_FLAG(top->flags, EXT_SUBTLV_LINK_ADJ_SID_VFLG) + ? SID_LABEL_SIZE(EXT_SUBTLV_ADJ_SID_SIZE) + : SID_INDEX_SIZE(EXT_SUBTLV_ADJ_SID_SIZE); + check_tlv_size(tlv_size, "Adjacency SID"); vty_out(vty, " Adj-SID Sub-TLV: Length %u\n\tFlags: " @@ -1632,8 +1636,12 @@ static uint16_t show_vty_ext_link_lan_adj_sid(struct vty *vty, { struct ext_subtlv_lan_adj_sid *top = (struct ext_subtlv_lan_adj_sid *)tlvh; + uint8_t tlv_size; - check_tlv_size(EXT_SUBTLV_LAN_ADJ_SID_SIZE, "Lan-Adjacency SID"); + tlv_size = CHECK_FLAG(top->flags, EXT_SUBTLV_LINK_ADJ_SID_VFLG) + ? SID_LABEL_SIZE(EXT_SUBTLV_LAN_ADJ_SID_SIZE) + : SID_INDEX_SIZE(EXT_SUBTLV_LAN_ADJ_SID_SIZE); + check_tlv_size(tlv_size, "LAN-Adjacency SID"); vty_out(vty, " LAN-Adj-SID Sub-TLV: Length %u\n\tFlags: " @@ -1742,8 +1750,12 @@ static uint16_t show_vty_ext_pref_pref_sid(struct vty *vty, { struct ext_subtlv_prefix_sid *top = (struct ext_subtlv_prefix_sid *)tlvh; + uint8_t tlv_size; - check_tlv_size(EXT_SUBTLV_PREFIX_SID_SIZE, "Prefix SID"); + tlv_size = CHECK_FLAG(top->flags, EXT_SUBTLV_PREFIX_SID_VFLG) + ? SID_LABEL_SIZE(EXT_SUBTLV_PREFIX_SID_SIZE) + : SID_INDEX_SIZE(EXT_SUBTLV_PREFIX_SID_SIZE); + check_tlv_size(tlv_size, "Prefix SID"); vty_out(vty, " Prefix SID Sub-TLV: Length %u\n\tAlgorithm: " -- 2.51.0 From 25d4a7185f4d1458e6bfc0d6927cc182f49609db Mon Sep 17 00:00:00 2001 From: Marius Tomaschewski Date: Fri, 13 Mar 2026 11:10:02 +0100 Subject: [PATCH 3/5] ospfd: Backport vty_out check in check_tlv_size Upstream: yes References: CVE-2025-61099,CVE-2025-61100,CVE-2025-61101,CVE-2025-61102,CVE-2025-61103,CVE-2025-61104,CVE-2025-61105,CVE-2025-61106,CVE-2025-61107,bsc#1252838,bsc#1252829,bsc#1252833,bsc#1252835,bsc#1252810,bsc#1252811,bsc#1252761,bsc#1252812,bsc#1252813 Applied commit b7d9b7aa47627b31e4b50795284408ab6de98660 from https://github.com/FRRouting/frr/pull/19983 ``` ospfd: Add null check for vty_out in check_tlv_size Add security check for vty_out. Specifically, Check NULL for vty. If vty is not available, dump info via zlog. Signed-off-by: s1awwhy ``` Signed-off-by: Marius Tomaschewski diff --git a/ospfd/ospf_ext.c b/ospfd/ospf_ext.c index 9e2ec89531..0b34b89148 100644 --- a/ospfd/ospf_ext.c +++ b/ospfd/ospf_ext.c @@ -1579,11 +1579,15 @@ static void ospf_ext_lsa_schedule(struct ext_itf *exti, enum lsa_opcode op) * ------------------------------------ */ +/* Check NULL for vty. If vty is not available, dump info via zlog */ #define check_tlv_size(size, msg) \ do { \ if (ntohs(tlvh->length) != size) { \ - vty_out(vty, " Wrong %s TLV size: %d(%d). Abort!\n", \ - msg, ntohs(tlvh->length), size); \ + if (vty != NULL) \ + vty_out(vty, " Wrong %s TLV size: %d(%d). Abort!\n", \ + msg, ntohs(tlvh->length), size); \ + else \ + zlog_debug(" Wrong %s TLV size: %d(%d). Abort!", msg, ntohs(tlvh->length), size); \ return size + TLV_HDR_SIZE; \ } \ } while (0) -- 2.51.0 From 385ce2535049f30ce567e0b93f093bf9c8ead773 Mon Sep 17 00:00:00 2001 From: Marius Tomaschewski Date: Fri, 13 Mar 2026 11:10:23 +0100 Subject: [PATCH 4/5] ospfd: Backport NULL Pointer Deference fixes Upstream: yes References: CVE-2025-61099,CVE-2025-61100,CVE-2025-61101,CVE-2025-61102,CVE-2025-61103,CVE-2025-61104,CVE-2025-61105,CVE-2025-61106,CVE-2025-61107,bsc#1252838,bsc#1252829,bsc#1252833,bsc#1252835,bsc#1252810,bsc#1252811,bsc#1252761,bsc#1252812,bsc#1252813 Backport from commit 034e6fe67078810b952630055614ee5710d1196e in https://github.com/FRRouting/frr/pull/19983 ``` ospfd: Fix NULL Pointer Deference when dumping link info When the command debug ospf packet all send/recv detail is enabled in the OSPF configuration, ospfd will dump detailed information of any received or sent OSPF packets, either via VTY or through the zlog. However, the original Opaque LSA handling code failed to check whether the VTY context and show_opaque_info were available, resulting in NULL pointer dereference and crashes in ospfd. The patch fixes the Null Pointer Deference Vulnerability in show_vty_ext_link_rmt_itf_addr, show_vty_ext_link_adj_sid, show_vty_ext_link_lan_adj_sid, show_vty_unknown_tlv, show_vty_link_info, show_vty_ext_pref_pref_sid, show_vtY_pref_info. Specifically, add NULL check for vty. If vty is not available, dump details via zlog. Signed-off-by: s1awwhy Signed-off-by: Louis Scalbert ``` Signed-off-by: Marius Tomaschewski diff --git a/ospfd/ospf_ext.c b/ospfd/ospf_ext.c index 0b34b89148..8f8a27696e 100644 --- a/ospfd/ospf_ext.c +++ b/ospfd/ospf_ext.c @@ -1601,10 +1601,16 @@ static uint16_t show_vty_ext_link_rmt_itf_addr(struct vty *vty, check_tlv_size(EXT_SUBTLV_RMT_ITF_ADDR_SIZE, "Remote Itf. Address"); - vty_out(vty, - " Remote Interface Address Sub-TLV: Length %u\n " - "Address: %s\n", - ntohs(top->header.length), inet_ntoa(top->value)); + if (vty != NULL) { + /* Add security check for vty_out. If vty is not available, dump info via zlog.*/ + vty_out(vty, + " Remote Interface Address Sub-TLV: Length %u\n Address: %pI4\n", + ntohs(top->header.length), &top->value); + } else { + zlog_debug(" Remote Interface Address Sub-TLV: Length %u", + ntohs(top->header.length)); + zlog_debug(" Address: %pI4", &top->value); + } return TLV_SIZE(tlvh); } @@ -1621,15 +1627,29 @@ static uint16_t show_vty_ext_link_adj_sid(struct vty *vty, : SID_INDEX_SIZE(EXT_SUBTLV_ADJ_SID_SIZE); check_tlv_size(tlv_size, "Adjacency SID"); - vty_out(vty, - " Adj-SID Sub-TLV: Length %u\n\tFlags: " - "0x%x\n\tMT-ID:0x%x\n\tWeight: 0x%x\n\t%s: %u\n", - ntohs(top->header.length), top->flags, top->mtid, top->weight, - CHECK_FLAG(top->flags, EXT_SUBTLV_LINK_ADJ_SID_VFLG) ? "Label" - : "Index", - CHECK_FLAG(top->flags, EXT_SUBTLV_LINK_ADJ_SID_VFLG) - ? GET_LABEL(ntohl(top->value)) - : ntohl(top->value)); + if (vty != NULL) { + /* Add security check for vty_out. If vty is not available, dump info via zlog.*/ + vty_out(vty, + " Adj-SID Sub-TLV: Length %u\n\tFlags: " + "0x%x\n\tMT-ID:0x%x\n\tWeight: 0x%x\n\t%s: %u\n", + ntohs(top->header.length), top->flags, top->mtid, top->weight, + CHECK_FLAG(top->flags, EXT_SUBTLV_LINK_ADJ_SID_VFLG) ? "Label" + : "Index", + CHECK_FLAG(top->flags, EXT_SUBTLV_LINK_ADJ_SID_VFLG) + ? GET_LABEL(ntohl(top->value)) + : ntohl(top->value)); + } else { + zlog_debug(" Adj-SID Sub-TLV: Length %u", ntohs(top->header.length)); + zlog_debug(" Flags: 0x%x", top->flags); + zlog_debug(" MT-ID:0x%x", top->mtid); + zlog_debug(" Weight: 0x%x", top->weight); + zlog_debug(" %s: %u", + CHECK_FLAG(top->flags, EXT_SUBTLV_LINK_ADJ_SID_VFLG) ? "Label" + : "Index", + CHECK_FLAG(top->flags, EXT_SUBTLV_LINK_ADJ_SID_VFLG) + ? GET_LABEL(ntohl(top->value)) + : ntohl(top->value)); + } return TLV_SIZE(tlvh); } @@ -1647,17 +1667,32 @@ static uint16_t show_vty_ext_link_lan_adj_sid(struct vty *vty, : SID_INDEX_SIZE(EXT_SUBTLV_LAN_ADJ_SID_SIZE); check_tlv_size(tlv_size, "LAN-Adjacency SID"); - vty_out(vty, - " LAN-Adj-SID Sub-TLV: Length %u\n\tFlags: " - "0x%x\n\tMT-ID:0x%x\n\tWeight: 0x%x\n\tNeighbor ID: " - "%s\n\t%s: %u\n", - ntohs(top->header.length), top->flags, top->mtid, top->weight, - inet_ntoa(top->neighbor_id), - CHECK_FLAG(top->flags, EXT_SUBTLV_LINK_ADJ_SID_VFLG) ? "Label" - : "Index", - CHECK_FLAG(top->flags, EXT_SUBTLV_LINK_ADJ_SID_VFLG) - ? GET_LABEL(ntohl(top->value)) - : ntohl(top->value)); + if (vty != NULL) { + /* Add security check for vty_out. If vty is not available, dump info via zlog.*/ + vty_out(vty, + " LAN-Adj-SID Sub-TLV: Length %u\n\tFlags: " + "0x%x\n\tMT-ID:0x%x\n\tWeight: 0x%x\n\tNeighbor ID: " + "%s\n\t%s: %u\n", + ntohs(top->header.length), top->flags, top->mtid, top->weight, + inet_ntoa(top->neighbor_id), + CHECK_FLAG(top->flags, EXT_SUBTLV_LINK_ADJ_SID_VFLG) ? "Label" + : "Index", + CHECK_FLAG(top->flags, EXT_SUBTLV_LINK_ADJ_SID_VFLG) + ? GET_LABEL(ntohl(top->value)) + : ntohl(top->value)); + } else { + zlog_debug(" LAN-Adj-SID Sub-TLV: Length %u", ntohs(top->header.length)); + zlog_debug(" Flags: 0x%x", top->flags); + zlog_debug(" MT-ID:0x%x", top->mtid); + zlog_debug(" Weight: 0x%x", top->weight); + zlog_debug(" Neighbor ID: %pI4", &top->neighbor_id); + zlog_debug(" %s: %u", + CHECK_FLAG(top->flags, EXT_SUBTLV_LINK_ADJ_SID_VFLG) ? "Label" + : "Index", + CHECK_FLAG(top->flags, EXT_SUBTLV_LINK_ADJ_SID_VFLG) + ? GET_LABEL(ntohl(top->value)) + : ntohl(top->value)); + } return TLV_SIZE(tlvh); } @@ -1665,14 +1700,23 @@ static uint16_t show_vty_ext_link_lan_adj_sid(struct vty *vty, static uint16_t show_vty_unknown_tlv(struct vty *vty, struct tlv_header *tlvh, size_t buf_size) { + /* Add security check for vty_out. If vty is not available, dump info via zlog. */ if (TLV_SIZE(tlvh) > buf_size) { - vty_out(vty, " TLV size %d exceeds buffer size. Abort!", - TLV_SIZE(tlvh)); + if (vty != NULL) + vty_out(vty, " TLV size %d exceeds buffer size. Abort!", TLV_SIZE(tlvh)); + else + zlog_debug(" TLV size %d exceeds buffer size. Abort!", TLV_SIZE(tlvh)); + return buf_size; } - vty_out(vty, " Unknown TLV: [type(0x%x), length(0x%x)]\n", - ntohs(tlvh->type), ntohs(tlvh->length)); + if (vty != NULL) { + vty_out(vty, " Unknown TLV: [type(0x%x), length(0x%x)]\n", + ntohs(tlvh->type), ntohs(tlvh->length)); + } else { + zlog_debug(" Unknown TLV: [type(0x%x), length(0x%x)]", + ntohs(tlvh->type), ntohs(tlvh->length)); + } return TLV_SIZE(tlvh); } @@ -1688,18 +1732,32 @@ static uint16_t show_vty_link_info(struct vty *vty, struct tlv_header *ext, /* Verify that TLV length is valid against remaining buffer size */ if (length > buf_size) { - vty_out(vty, - " Extended Link TLV size %d exceeds buffer size. Abort!\n", - length); - return buf_size; + /* Add security check for vty_out. If vty is not available, dump info via zlog. */ + if (vty != NULL) { + vty_out(vty, + " Extended Link TLV size %d exceeds buffer size. Abort!\n", + length); + } else { + zlog_debug(" Extended Link TLV size %d exceeds buffer size. Abort!", + length); + } + return buf_size; } - vty_out(vty, - " Extended Link TLV: Length %u\n Link Type: 0x%x\n" - " Link ID: %s\n", - ntohs(top->header.length), top->link_type, - inet_ntoa(top->link_id)); - vty_out(vty, " Link data: %s\n", inet_ntoa(top->link_data)); + /* Add security check for vty_out. If vty is not available, dump info via zlog. */ + if (vty != NULL) { + vty_out(vty, + " Extended Link TLV: Length %u\n Link Type: 0x%x\n" + " Link ID: %s\n", + ntohs(top->header.length), top->link_type, + inet_ntoa(top->link_id)); + vty_out(vty, " Link data: %s\n", inet_ntoa(top->link_data)); + } else { + zlog_debug(" Extended Link TLV: Length %u", ntohs(top->header.length)); + zlog_debug(" Link Type: 0x%x", top->link_type); + zlog_debug(" Link ID: %pI4", &top->link_id); + zlog_debug(" Link data: %pI4", &top->link_data); + } /* Skip Extended TLV and parse sub-TLVs */ length -= EXT_TLV_LINK_SIZE; @@ -1761,16 +1819,30 @@ static uint16_t show_vty_ext_pref_pref_sid(struct vty *vty, : SID_INDEX_SIZE(EXT_SUBTLV_PREFIX_SID_SIZE); check_tlv_size(tlv_size, "Prefix SID"); - vty_out(vty, - " Prefix SID Sub-TLV: Length %u\n\tAlgorithm: " - "%u\n\tFlags: 0x%x\n\tMT-ID:0x%x\n\t%s: %u\n", - ntohs(top->header.length), top->algorithm, top->flags, - top->mtid, - CHECK_FLAG(top->flags, EXT_SUBTLV_PREFIX_SID_VFLG) ? "Label" - : "Index", - CHECK_FLAG(top->flags, EXT_SUBTLV_PREFIX_SID_VFLG) - ? GET_LABEL(ntohl(top->value)) - : ntohl(top->value)); + /* Add security check for vty_out. If vty is not available, dump info via zlog. */ + if (vty != NULL) { + vty_out(vty, + " Prefix SID Sub-TLV: Length %u\n\tAlgorithm: " + "%u\n\tFlags: 0x%x\n\tMT-ID:0x%x\n\t%s: %u\n", + ntohs(top->header.length), top->algorithm, top->flags, + top->mtid, + CHECK_FLAG(top->flags, EXT_SUBTLV_PREFIX_SID_VFLG) ? "Label" + : "Index", + CHECK_FLAG(top->flags, EXT_SUBTLV_PREFIX_SID_VFLG) + ? GET_LABEL(ntohl(top->value)) + : ntohl(top->value)); + } else { + zlog_debug(" Prefix SID Sub-TLV: Length %u", ntohs(top->header.length)); + zlog_debug(" Algorithm: %u", top->algorithm); + zlog_debug(" Flags: 0x%x", top->flags); + zlog_debug(" MT-ID:0x%x", top->mtid); + zlog_debug(" %s: %u", + CHECK_FLAG(top->flags, EXT_SUBTLV_PREFIX_SID_VFLG) ? "Label" + : "Index", + CHECK_FLAG(top->flags, EXT_SUBTLV_PREFIX_SID_VFLG) + ? GET_LABEL(ntohl(top->value)) + : ntohl(top->value)); + } return TLV_SIZE(tlvh); } @@ -1786,17 +1858,32 @@ static uint16_t show_vty_pref_info(struct vty *vty, struct tlv_header *ext, /* Verify that TLV length is valid against remaining buffer size */ if (length > buf_size) { - vty_out(vty, - " Extended Link TLV size %d exceeds buffer size. Abort!\n", - length); + /* Add security check for vty_out. If vty is not available, dump info via zlog. */ + if (vty != NULL) { + vty_out(vty, + " Extended Link TLV size %d exceeds buffer size. Abort!\n", + length); + } else { + zlog_debug(" Extended Link TLV size %d exceeds buffer size. Abort!", + length); + } return buf_size; } - vty_out(vty, - " Extended Prefix TLV: Length %u\n\tRoute Type: %u\n" - "\tAddress Family: 0x%x\n\tFlags: 0x%x\n\tAddress: %s/%u\n", - ntohs(top->header.length), top->route_type, top->af, top->flags, - inet_ntoa(top->address), top->pref_length); + /* Add security check for vty_out. If vty is not available, dump info via zlog. */ + if (vty != NULL) { + vty_out(vty, + " Extended Prefix TLV: Length %u\n\tRoute Type: %u\n" + "\tAddress Family: 0x%x\n\tFlags: 0x%x\n\tAddress: %pI4/%u\n", + ntohs(top->header.length), top->route_type, top->af, top->flags, + &top->address, top->pref_length); + } else { + zlog_debug(" Extended Prefix TLV: Length %u", ntohs(top->header.length)); + zlog_debug(" Route Type: %u", top->route_type); + zlog_debug(" Address Family: 0x%x", top->af); + zlog_debug(" Flags: 0x%x", top->flags); + zlog_debug(" Address: %pI4/%u", &top->address, top->pref_length); + } /* Skip Extended Prefix TLV and parse sub-TLVs */ length -= EXT_TLV_PREFIX_SIZE; -- 2.51.0 From 91320eef419fcf8c5739009e0a757debcf9a55bc Mon Sep 17 00:00:00 2001 From: Marius Tomaschewski Date: Fri, 13 Mar 2026 11:11:27 +0100 Subject: [PATCH 5/5] ospfd: Backport to skip invalid length tlvs Upstream: yes References: CVE-2025-61099,CVE-2025-61100,CVE-2025-61101,CVE-2025-61102,CVE-2025-61103,CVE-2025-61104,CVE-2025-61105,CVE-2025-61106,CVE-2025-61107,bsc#1252838,bsc#1252829,bsc#1252833,bsc#1252835,bsc#1252810,bsc#1252811,bsc#1252761,bsc#1252812,bsc#1252813 Backport from commit 33dfc7e7be1ac8b66abbf47c30a709215fbc1926 in https://github.com/FRRouting/frr/pull/19983 ``` ospfd: skip subsequent tlvs after invalid length Do not attempt to read subsequent TLVs after an TLV invalid length is detected. Signed-off-by: Louis Scalbert ``` Signed-off-by: Marius Tomaschewski diff --git a/ospfd/ospf_ext.c b/ospfd/ospf_ext.c index 8f8a27696e..be40ee0488 100644 --- a/ospfd/ospf_ext.c +++ b/ospfd/ospf_ext.c @@ -1584,11 +1584,12 @@ static void ospf_ext_lsa_schedule(struct ext_itf *exti, enum lsa_opcode op) do { \ if (ntohs(tlvh->length) != size) { \ if (vty != NULL) \ - vty_out(vty, " Wrong %s TLV size: %d(%d). Abort!\n", \ + vty_out(vty, " Wrong %s TLV size: %d(expected %d). Skip subsequent TLVs!\n", \ msg, ntohs(tlvh->length), size); \ else \ - zlog_debug(" Wrong %s TLV size: %d(%d). Abort!", msg, ntohs(tlvh->length), size); \ - return size + TLV_HDR_SIZE; \ + zlog_debug(" Wrong %s TLV size: %d(expected %d). Skip subsequent TLVs!", \ + msg, ntohs(tlvh->length), size); \ + return OSPF_MAX_LSA_SIZE + 1; \ } \ } while (0) diff --git a/ospfd/ospf_ri.c b/ospfd/ospf_ri.c index 2589b99fef..9d55097067 100644 --- a/ospfd/ospf_ri.c +++ b/ospfd/ospf_ri.c @@ -1188,12 +1188,12 @@ static int ospf_router_info_lsa_update(struct ospf_lsa *lsa) do { \ if (ntohs(tlvh->length) > size) { \ if (vty != NULL) \ - vty_out(vty, " Wrong %s TLV size: %d(%d)\n", \ + vty_out(vty, " Wrong %s TLV size: %d(expected %d). Skip subsequent TLVs!\n", \ msg, ntohs(tlvh->length), size); \ else \ - zlog_debug(" Wrong %s TLV size: %d(%d)\n", \ + zlog_debug(" Wrong %s TLV size: %d(expected %d). Skip subsequent TLVs!", \ msg, ntohs(tlvh->length), size); \ - return size + TLV_HDR_SIZE; \ + return OSPF_MAX_LSA_SIZE + 1; \ } \ } while (0) diff --git a/ospfd/ospf_te.c b/ospfd/ospf_te.c index b078ac7400..06bfb1d25b 100644 --- a/ospfd/ospf_te.c +++ b/ospfd/ospf_te.c @@ -1591,12 +1591,12 @@ void ospf_mpls_te_lsa_schedule(struct mpls_te_link *lp, enum lsa_opcode opcode) do { \ if (ntohs(tlvh->length) > size) { \ if (vty != NULL) \ - vty_out(vty, " Wrong %s TLV size: %d(%d)\n", \ + vty_out(vty, " Wrong %s TLV size: %d(expected %d). Skip subsequent TLVs!\n", \ msg, ntohs(tlvh->length), size); \ else \ - zlog_debug(" Wrong %s TLV size: %d(%d)\n", \ + zlog_debug(" Wrong %s TLV size: %d(expected %d). Skip subsequent TLVs!", \ msg, ntohs(tlvh->length), size); \ - return size + TLV_HDR_SIZE; \ + return OSPF_MAX_LSA_SIZE + 1; \ } \ } while(0) -- 2.51.0