Commit 223e1d935c for frr
commit 223e1d935c033da887da27c78b64442d81551df1
Author: Alejandro Pacheco <alepacheco99@gmail.com>
Date: Wed Oct 14 15:57:36 2026 +0200
ospf6d: check grace LSA TLV sizes before reading their values
0ae847c287 made the grace LSA TLV loop check that a TLV header is
present, but a Grace Period or Restart Reason TLV is still read as its
full struct whatever its length field says. A TLV with a short length
at the end of the LSA makes ospf6_extract_grace_lsa_fields() read past
the LSA buffer; one followed by another TLV takes the interval or the
reason from that TLV's header and is accepted.
The show code walks the same TLVs with weaker checks: it reads a TLV
header with fewer than four bytes left, does not step over unknown
TLVs (so it keeps walking past the end of the LSA), and indexes the
restart reason table with the received reason. It runs for
"show ipv6 ospf6 database ... detail", including for MaxAge grace LSAs
that were installed without going through the helper's checks.
Check every grace TLV in one helper, used by both loops, that requires
the TLV to fit in the LSA and to be at least as long as its type's
struct, as ospfd does in ospf_extract_grace_lsa_fields(). The show loop
now needs a whole TLV header and steps over unknown TLVs, and the
restart reason string comes from a bounds-checked lookup.
tests/ospf6d/test_grace_lsa covers short Grace Period and Restart
Reason TLVs before another TLV and at the end of the LSA, and the show
path (through ospf6_lsa_show() with JSON output) with an unknown TLV
and an unknown restart reason. Each LSA is followed in memory by bytes
the test controls, so every case fails on master without a sanitizer.
grace_lsa_handler is declared in ospf6_gr.h for the test.
Fixes: 0d1753a7db ("ospf6d: Helper functionality changes")
Fixes: 0fc3e11323 ("ospf6d: GR helper configurations")
Signed-off-by: Alejandro Pacheco <alepacheco99@gmail.com>
diff --git a/ospf6d/ospf6_gr.h b/ospf6d/ospf6_gr.h
index 22ad2f1c21..3970a12145 100644
--- a/ospf6d/ospf6_gr.h
+++ b/ospf6d/ospf6_gr.h
@@ -98,6 +98,8 @@ extern const char *ospf6_exit_reason_desc[];
extern const char *ospf6_restart_reason_desc[];
extern const char *ospf6_rejected_reason_desc[];
+extern struct ospf6_lsa_handler grace_lsa_handler;
+
extern void ospf6_gr_helper_config_init(void);
extern void ospf6_gr_helper_init(struct ospf6 *ospf6);
extern void ospf6_gr_helper_deinit(struct ospf6 *ospf6);
diff --git a/ospf6d/ospf6_gr_helper.c b/ospf6d/ospf6_gr_helper.c
index 4be9be0be2..a3d63c2ad6 100644
--- a/ospf6d/ospf6_gr_helper.c
+++ b/ospf6d/ospf6_gr_helper.c
@@ -115,6 +115,41 @@ static void ospf6_enable_rtr_hash_destroy(struct ospf6 *ospf6)
ospf6_disable_rtr_hash_free);
}
+/*
+ * Check the grace LSA TLV at offset sum of a TLV area of length bytes:
+ * the whole TLV must fit in the area and be long enough for the fields
+ * its type carries.
+ */
+static bool ospf6_grace_tlv_valid(const struct tlv_header *tlvh, int sum, uint16_t length)
+{
+ uint32_t min_size;
+
+ if (sum + TLV_SIZE(tlvh) > length)
+ return false;
+
+ switch (ntohs(tlvh->type)) {
+ case TLV_GRACE_PERIOD_TYPE:
+ min_size = GRACE_PERIOD_TLV_SIZE;
+ break;
+ case TLV_GRACE_RESTART_REASON_TYPE:
+ min_size = GRACE_RESTART_REASON_TLV_SIZE;
+ break;
+ default:
+ min_size = TLV_HDR_SIZE;
+ break;
+ }
+
+ return TLV_SIZE(tlvh) >= min_size;
+}
+
+static const char *ospf6_restart_reason_str(uint8_t reason)
+{
+ if (reason >= array_size(ospf6_restart_reason_desc))
+ return ospf6_restart_reason_desc[0];
+
+ return ospf6_restart_reason_desc[reason];
+}
+
/*
* Extracting tlv info from GRACE LSA.
*
@@ -148,8 +183,8 @@ static int ospf6_extract_grace_lsa_fields(struct ospf6_lsa *lsa,
/* Iterate as long as we have at least a TLV header available */
for (tlvh = lsdesc_start(lsah); sum + TLV_HDR_SIZE <= length && tlvh;
tlvh = TLV_HDR_NEXT(tlvh)) {
- /* Check TLV len against overall LSA */
- if (sum + TLV_SIZE(tlvh) > length) {
+ /* Check TLV len against overall LSA and its type */
+ if (!ospf6_grace_tlv_valid(tlvh, sum, length)) {
if (IS_DEBUG_OSPF6_GR)
zlog_debug(
"%s: Malformed packet: Invalid TLV len:%d",
@@ -1243,10 +1278,10 @@ static int ospf6_grace_lsa_show_info(struct vty *vty, struct ospf6_lsa *lsa,
zlog_debug(" TLV info:");
}
- for (tlvh = lsdesc_start(lsah); sum < length && tlvh;
+ for (tlvh = lsdesc_start(lsah); sum + TLV_HDR_SIZE <= length && tlvh;
tlvh = TLV_HDR_NEXT(tlvh)) {
- /* Check TLV len */
- if (sum + TLV_SIZE(tlvh) > length) {
+ /* Check TLV len against overall LSA and its type */
+ if (!ospf6_grace_tlv_valid(tlvh, sum, length)) {
if (vty)
vty_out(vty, "%% Invalid TLV length: %d\n",
TLV_SIZE(tlvh));
@@ -1279,21 +1314,19 @@ static int ospf6_grace_lsa_show_info(struct vty *vty, struct ospf6_lsa *lsa,
sum += TLV_SIZE(tlvh);
if (vty) {
if (use_json)
- json_object_string_add(
- json, "restartReason",
- ospf6_restart_reason_desc
- [grReason->reason]);
+ json_object_string_add(json, "restartReason",
+ ospf6_restart_reason_str(
+ grReason->reason));
else
vty_out(vty, " Restart reason:%s\n",
- ospf6_restart_reason_desc
- [grReason->reason]);
+ ospf6_restart_reason_str(grReason->reason));
} else {
zlog_debug(" Restart reason:%s",
- ospf6_restart_reason_desc
- [grReason->reason]);
+ ospf6_restart_reason_str(grReason->reason));
}
break;
default:
+ sum += TLV_SIZE(tlvh);
break;
}
}
diff --git a/tests/.gitignore b/tests/.gitignore
index 975d67ab6f..8fc6c01a8a 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -83,6 +83,7 @@ frr_northbound*
/lib/test_zlog
/lib/test_zmq
/ospfd/test_ospf_asbr_summary
+/ospf6d/test_grace_lsa
/ospf6d/test_lsdb
/ospf6d/test_lsdb_clippy.c
/pimd/test_pim_jp_encode
diff --git a/tests/ospf6d/subdir.am b/tests/ospf6d/subdir.am
index ef1f40cf08..0bbd3a51d4 100644
--- a/tests/ospf6d/subdir.am
+++ b/tests/ospf6d/subdir.am
@@ -12,7 +12,17 @@ tests_ospf6d_test_lsdb_CPPFLAGS = $(TESTS_CPPFLAGS)
tests_ospf6d_test_lsdb_LDADD = $(OSPF6_TEST_LDADD)
tests_ospf6d_test_lsdb_SOURCES = tests/ospf6d/test_lsdb.c tests/lib/cli/common_cli.c
clippy_scan += tests/ospf6d/test_lsdb.c
+
+if OSPF6D
+check_PROGRAMS += tests/ospf6d/test_grace_lsa
+endif
+tests_ospf6d_test_grace_lsa_CFLAGS = $(TESTS_CFLAGS)
+tests_ospf6d_test_grace_lsa_CPPFLAGS = $(TESTS_CPPFLAGS)
+tests_ospf6d_test_grace_lsa_LDADD = $(OSPF6_TEST_LDADD)
+tests_ospf6d_test_grace_lsa_SOURCES = tests/ospf6d/test_grace_lsa.c
+
EXTRA_DIST += \
+ tests/ospf6d/test_grace_lsa.py \
tests/ospf6d/test_lsdb.py \
tests/ospf6d/test_lsdb.in \
tests/ospf6d/test_lsdb.refout \
diff --git a/tests/ospf6d/test_grace_lsa.c b/tests/ospf6d/test_grace_lsa.c
new file mode 100644
index 0000000000..e2adfc758a
--- /dev/null
+++ b/tests/ospf6d/test_grace_lsa.c
@@ -0,0 +1,228 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Grace-LSA TLV checks: a Grace Period or Restart Reason TLV shorter than
+ * its value is rejected rather than read past its end, and the show code
+ * stays within the LSA and prints only known restart reasons.
+ *
+ * Each LSA is followed in memory by bytes the test controls, so reading
+ * past its end changes the result instead of depending on what follows
+ * an allocation.
+ */
+
+#include <zebra.h>
+
+#include "memory.h"
+#include "privs.h"
+#include "frrevent.h"
+#include "libospf.h"
+#include "vty.h"
+#include "json.h"
+
+#include "ospf6d/ospf6_proto.h"
+#include "ospf6d/ospf6_lsa.h"
+#include "ospf6d/ospf6_top.h"
+#include "ospf6d/ospf6_neighbor.h"
+#include "ospf6d/ospf6_tlv.h"
+#include "ospf6d/ospf6_gr.h"
+#include "ospf6d/ospf6d.h"
+
+#define TEST_VALID "grace-lsa: valid grace period and restart reason"
+#define TEST_PERIOD_SHORT "grace-lsa: empty grace period TLV before another TLV"
+#define TEST_REASON_SHORT "grace-lsa: empty restart reason TLV before another TLV"
+#define TEST_PERIOD_END "grace-lsa: empty grace period TLV at the end of the LSA"
+#define TEST_SHOW_UNKNOWN "grace-lsa: show steps over an unknown TLV"
+#define TEST_SHOW_REASON "grace-lsa: show an unknown restart reason"
+
+#define BODY_MAX 256
+
+struct event_loop *master;
+struct zebra_privs_t ospf6d_privs;
+
+static struct ospf6 *ospf6;
+static struct ospf6_neighbor *restarter;
+
+static void failed(const char *test, const char *why)
+{
+ printf("%s\n%s\nfailed!\n", test, why);
+ exit(1);
+}
+
+/*
+ * A grace LSA in buf with the given body, followed by the given bytes,
+ * which are not part of the LSA.
+ */
+static void grace_lsa(struct ospf6_lsa *lsa, uint8_t *buf, const uint8_t *body, size_t len,
+ const uint8_t *after, size_t after_len)
+{
+ struct ospf6_lsa_header *lsah = (struct ospf6_lsa_header *)buf;
+
+ assert(len + after_len <= BODY_MAX);
+ memset(buf, 0, OSPF6_LSA_HEADER_SIZE + BODY_MAX);
+ lsah->type = htons(OSPF6_LSTYPE_GRACE_LSA);
+ lsah->adv_router = htonl(0x02020202);
+ lsah->seqnum = htonl(OSPF_INITIAL_SEQUENCE_NUMBER);
+ lsah->length = htons(OSPF6_LSA_HEADER_SIZE + len);
+ memcpy(buf + OSPF6_LSA_HEADER_SIZE, body, len);
+ if (after_len)
+ memcpy(buf + OSPF6_LSA_HEADER_SIZE + len, after, after_len);
+
+ memset(lsa, 0, sizeof(*lsa));
+ lsa->header = lsah;
+}
+
+/*
+ * Process a grace LSA from a neighbor that is not FULL: an LSA whose TLVs
+ * are accepted is rejected for the neighbor's state, a malformed one
+ * before that.
+ */
+static bool grace_lsa_accepted(const uint8_t *body, size_t len, const uint8_t *after,
+ size_t after_len)
+{
+ uint8_t buf[OSPF6_LSA_HEADER_SIZE + BODY_MAX];
+ struct ospf6_lsa lsa;
+ int ret;
+
+ grace_lsa(&lsa, buf, body, len, after, after_len);
+ restarter->gr_helper_info.rejected_reason = OSPF6_HELPER_REJECTED_NONE;
+ ret = ospf6_process_grace_lsa(ospf6, &lsa, restarter);
+
+ assert(ret == OSPF6_GR_NOT_HELPER);
+ return restarter->gr_helper_info.rejected_reason == OSPF6_HELPER_NOT_A_VALID_NEIGHBOUR;
+}
+
+/* The restart reason "show ipv6 ospf6 database ... json" prints. */
+static void grace_lsa_show_reason(const uint8_t *body, size_t len, const uint8_t *after,
+ size_t after_len, char *reason, size_t reason_size)
+{
+ uint8_t buf[OSPF6_LSA_HEADER_SIZE + BODY_MAX];
+ struct ospf6_lsa lsa;
+ struct vty *vty = vty_new();
+ json_object *json = json_object_new_array();
+ json_object *jreason;
+
+ vty->type = VTY_TERM;
+ grace_lsa(&lsa, buf, body, len, after, after_len);
+ ospf6_lsa_show(vty, &lsa, json, true);
+
+ reason[0] = '\0';
+ if (json_object_object_get_ex(json_object_array_get_idx(json, 0), "restartReason",
+ &jreason))
+ strlcpy(reason, json_object_get_string(jreason), reason_size);
+
+ json_object_free(json);
+ vty_close(vty);
+}
+
+static void test_valid(void)
+{
+ const uint8_t body[] = {
+ 0, 1, 0, 4, 0, 0, 0, 120, /* grace period 120 */
+ 0, 2, 0, 1, 1, 0, 0, 0, /* restart reason 1 */
+ };
+
+ if (!grace_lsa_accepted(body, sizeof(body), NULL, 0))
+ failed(TEST_VALID, "valid grace LSA rejected");
+ printf("%s\nOK\n", TEST_VALID);
+}
+
+static void test_period_short(void)
+{
+ /*
+ * The grace period TLV has no value; the 4 bytes after its header
+ * are the next TLV's header, which reads as a valid interval.
+ */
+ uint8_t body[4 + 4 + 120] = {
+ 0, 1, 0, 0, /* grace period, length 0 */
+ 0, 0, 0, 120, /* unknown TLV, length 120 */
+ };
+
+ if (grace_lsa_accepted(body, sizeof(body), NULL, 0))
+ failed(TEST_PERIOD_SHORT, "short grace period TLV accepted");
+ printf("%s\nOK\n", TEST_PERIOD_SHORT);
+}
+
+static void test_reason_short(void)
+{
+ /* The reason byte is the next TLV's first byte. */
+ const uint8_t body[] = {
+ 0, 1, 0, 4, 0, 0, 0, 120, /* grace period 120 */
+ 0, 2, 0, 0, /* restart reason, length 0 */
+ 1, 0, 0, 0, /* unknown TLV, length 0 */
+ };
+
+ if (grace_lsa_accepted(body, sizeof(body), NULL, 0))
+ failed(TEST_REASON_SHORT, "short restart reason TLV accepted");
+ printf("%s\nOK\n", TEST_REASON_SHORT);
+}
+
+static void test_period_end(void)
+{
+ /* The interval would be the 4 bytes after the LSA. */
+ const uint8_t body[] = {
+ 0, 2, 0, 1, 1, 0, 0, 0, /* restart reason 1 */
+ 0, 1, 0, 0, /* grace period, length 0 */
+ };
+ const uint8_t after[] = { 0, 0, 0, 120 };
+
+ if (grace_lsa_accepted(body, sizeof(body), after, sizeof(after)))
+ failed(TEST_PERIOD_END, "grace period read after the LSA");
+ printf("%s\nOK\n", TEST_PERIOD_END);
+}
+
+static void test_show_unknown(void)
+{
+ /* An unknown TLV takes room in the LSA like any other. */
+ const uint8_t body[] = {
+ 0, 9, 0, 4, 0, 0, 0, 0, /* unknown TLV, length 4 */
+ 0, 2, 0, 1, 1, 0, 0, 0, /* restart reason 1 */
+ };
+ const uint8_t after[] = {
+ 0, 2, 0, 1, 3, 0, 0, 0, /* restart reason 3 */
+ };
+ char reason[64];
+
+ grace_lsa_show_reason(body, sizeof(body), after, sizeof(after), reason, sizeof(reason));
+ if (strcmp(reason, "Software restart"))
+ failed(TEST_SHOW_UNKNOWN, "TLV after the LSA shown");
+ printf("%s\nOK\n", TEST_SHOW_UNKNOWN);
+}
+
+static void test_show_reason(void)
+{
+ const uint8_t body[] = {
+ 0, 2, 0, 1, 200, 0, 0, 0, /* restart reason 200 */
+ };
+ char reason[64];
+
+ grace_lsa_show_reason(body, sizeof(body), NULL, 0, reason, sizeof(reason));
+ if (strcmp(reason, "Unknown restart"))
+ failed(TEST_SHOW_REASON, "unknown restart reason not shown as such");
+ printf("%s\nOK\n", TEST_SHOW_REASON);
+}
+
+int main(void)
+{
+ master = event_master_create(NULL);
+ ospf6_install_lsa_handler(&grace_lsa_handler);
+
+ ospf6 = XCALLOC(MTYPE_TMP, sizeof(*ospf6));
+ ospf6->router_id = htonl(0x01010101);
+ ospf6->ospf6_helper_cfg.is_helper_supported = true;
+
+ restarter = XCALLOC(MTYPE_TMP, sizeof(*restarter));
+ restarter->state = OSPF6_NEIGHBOR_TWOWAY;
+ restarter->router_id = htonl(0x02020202);
+ strlcpy(restarter->name, "2.2.2.2", sizeof(restarter->name));
+
+ test_valid();
+ test_period_short();
+ test_reason_short();
+ test_period_end();
+ test_show_unknown();
+ test_show_reason();
+
+ XFREE(MTYPE_TMP, restarter);
+ XFREE(MTYPE_TMP, ospf6);
+ event_master_free(master);
+ return 0;
+}
diff --git a/tests/ospf6d/test_grace_lsa.py b/tests/ospf6d/test_grace_lsa.py
new file mode 100644
index 0000000000..4054302c01
--- /dev/null
+++ b/tests/ospf6d/test_grace_lsa.py
@@ -0,0 +1,14 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+import frrtest
+
+
+class TestGraceLsa(frrtest.TestMultiOut):
+ program = "./test_grace_lsa"
+
+
+TestGraceLsa.okfail("grace-lsa: valid grace period and restart reason")
+TestGraceLsa.okfail("grace-lsa: empty grace period TLV before another TLV")
+TestGraceLsa.okfail("grace-lsa: empty restart reason TLV before another TLV")
+TestGraceLsa.okfail("grace-lsa: empty grace period TLV at the end of the LSA")
+TestGraceLsa.okfail("grace-lsa: show steps over an unknown TLV")
+TestGraceLsa.okfail("grace-lsa: show an unknown restart reason")