]> git.puffer.fish Git - matthieu/frr.git/commitdiff
ospfd: Correct Opaque LSA Extended parser
authorOlivier Dugeon <olivier.dugeon@orange.com>
Fri, 5 Apr 2024 10:57:11 +0000 (12:57 +0200)
committerMergify <37929162+mergify[bot]@users.noreply.github.com>
Fri, 24 May 2024 19:32:18 +0000 (19:32 +0000)
Iggy Frankovic discovered another ospfd crash when performing fuzzing of OSPF
LSA packets. The crash occurs in ospf_te_parse_ext_link() function when
attemping to read Segment Routing Adjacency SID subTLVs. The original code
doesn't check if the size of the Extended Link TLVs and subTLVs have the correct
length. In presence of erronous LSA, this will cause a buffer overflow and ospfd
crashes.

This patch introduces new verification of the subTLVs size for Extended Link
TLVs and subTLVs. Similar check has been also introduced for the Extended
Prefix TLV.

Co-authored-by: Iggy Frankovic <iggyfran@amazon.com>
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
(cherry picked from commit 5557a289acdaeec8cc63ffc97b5c2abf6dee7b3a)

ospfd/ospf_te.c

index c0353695e4a6a4d0a306bc9edea62667045b631b..bdb4a811f6bd56c8081c7938342f3099b677805a 100644 (file)
@@ -2620,6 +2620,7 @@ static int ospf_te_parse_ext_pref(struct ls_ted *ted, struct ospf_lsa *lsa)
        struct ext_tlv_prefix *ext;
        struct ext_subtlv_prefix_sid *pref_sid;
        uint32_t label;
+       uint16_t len, size;
 
        /* Get corresponding Subnet from Link State Data Base */
        ext = (struct ext_tlv_prefix *)TLV_HDR_TOP(lsa->data);
@@ -2641,6 +2642,18 @@ static int ospf_te_parse_ext_pref(struct ls_ted *ted, struct ospf_lsa *lsa)
        ote_debug("  |- Process Extended Prefix LSA %pI4 for subnet %pFX",
                  &lsa->data->id, &pref);
 
+       /*
+        * Check Extended Prefix TLV size against LSA size
+        * as only one TLV is allowed per LSA
+        */
+       len = TLV_BODY_SIZE(&ext->header);
+       size = lsa->size - (OSPF_LSA_HEADER_SIZE + TLV_HDR_SIZE);
+       if (len != size || len <= 0) {
+               ote_debug("  |- Wrong TLV size: %u instead of %u",
+                         (uint32_t)len, (uint32_t)size);
+               return -1;
+       }
+
        /* Initialize TLV browsing */
        ls_pref = subnet->ls_pref;
        pref_sid = (struct ext_subtlv_prefix_sid *)((char *)(ext) + TLV_HDR_SIZE
@@ -2751,8 +2764,20 @@ static int ospf_te_parse_ext_link(struct ls_ted *ted, struct ospf_lsa *lsa)
        ote_debug("  |- Process Extended Link LSA %pI4 for edge %pI4",
                  &lsa->data->id, &edge->attributes->standard.local);
 
-       /* Initialize TLV browsing */
-       len = TLV_BODY_SIZE(&ext->header) - EXT_TLV_LINK_SIZE;
+       /*
+        * Check Extended Link TLV size against LSA size
+        * as only one TLV is allowed per LSA
+        */
+       len = TLV_BODY_SIZE(&ext->header);
+       i = lsa->size - (OSPF_LSA_HEADER_SIZE + TLV_HDR_SIZE);
+       if (len != i || len <= 0) {
+               ote_debug("  |- Wrong TLV size: %u instead of %u",
+                         (uint32_t)len, (uint32_t)i);
+               return -1;
+       }
+
+       /* Initialize subTLVs browsing */
+       len -= EXT_TLV_LINK_SIZE;
        tlvh = (struct tlv_header *)((char *)(ext) + TLV_HDR_SIZE
                                     + EXT_TLV_LINK_SIZE);
        for (; sum < len; tlvh = TLV_HDR_NEXT(tlvh)) {
@@ -2762,6 +2787,8 @@ static int ospf_te_parse_ext_link(struct ls_ted *ted, struct ospf_lsa *lsa)
 
                switch (ntohs(tlvh->type)) {
                case EXT_SUBTLV_ADJ_SID:
+                       if (TLV_BODY_SIZE(tlvh) != EXT_SUBTLV_ADJ_SID_SIZE)
+                               break;
                        adj = (struct ext_subtlv_adj_sid *)tlvh;
                        label = CHECK_FLAG(adj->flags,
                                           EXT_SUBTLV_LINK_ADJ_SID_VFLG)
@@ -2788,6 +2815,8 @@ static int ospf_te_parse_ext_link(struct ls_ted *ted, struct ospf_lsa *lsa)
 
                        break;
                case EXT_SUBTLV_LAN_ADJ_SID:
+                       if (TLV_BODY_SIZE(tlvh) != EXT_SUBTLV_LAN_ADJ_SID_SIZE)
+                               break;
                        ladj = (struct ext_subtlv_lan_adj_sid *)tlvh;
                        label = CHECK_FLAG(ladj->flags,
                                           EXT_SUBTLV_LINK_ADJ_SID_VFLG)
@@ -2817,6 +2846,8 @@ static int ospf_te_parse_ext_link(struct ls_ted *ted, struct ospf_lsa *lsa)
 
                        break;
                case EXT_SUBTLV_RMT_ITF_ADDR:
+                       if (TLV_BODY_SIZE(tlvh) != EXT_SUBTLV_RMT_ITF_ADDR_SIZE)
+                               break;
                        rmt = (struct ext_subtlv_rmt_itf_addr *)tlvh;
                        if (CHECK_FLAG(atr->flags, LS_ATTR_NEIGH_ADDR)
                            && IPV4_ADDR_SAME(&atr->standard.remote,