isisd: fix rcap tlv double-free crash

A double-free crash happens when a subTLV of the "Router Capability"
TLV is not readable and a previous "Router Capability" TLV was read.

rcap was supposed to be freed later by isis_free_tlvs() ->
free_tlv_router_cap(). In 78774bbcd5 ("isisd: add isis flex-algo lsp
advertisement"), this was not the case because rcap was not saved to
tlvs->router_cap when the function returned early because of a subTLV
length issue.

Always set tlvs->router_cap to free the memory.

Note that this patch has the consequence that in case of subTLV error,
the previously read "Router Capability" subTLVs are kept in memory.

Fixes: 49efc80d34 ("isisd: Ensure rcap is freed in error case")
Fixes: 78774bbcd5 ("isisd: add isis flex-algo lsp advertisement")
Reported-by: Iggy Frankovic <iggyfran@amazon.com>
Signed-off-by: Louis Scalbert <louis.scalbert@6wind.com>
(cherry picked from commit d61758140d33972c10ecbb72d0a3e528049dd8d6)

diff --git a/isisd/isis_tlvs.c b/isisd/isis_tlvs.c
index 59d632d170edb9be8995236af647e8fc899a30c0..fe0bf4ae3e09548bd787a22707aa77eb4008e8b9 100644 (file)
--- a/isisd/isis_tlvs.c
+++ b/isisd/isis_tlvs.c
@@ -5270,16 +5270,17 @@ static int unpack_tlv_router_cap(enum isis_tlv_context context,
                return 0;
        }
 
-       if (tlvs->router_cap)
-               /* Multiple Router Capability found */
-               rcap = tlvs->router_cap;
-       else {
-               /* Allocate router cap structure and initialize SR Algorithms */
-               rcap = XCALLOC(MTYPE_ISIS_TLV, sizeof(struct isis_router_cap));
+       if (!tlvs->router_cap) {
+               /* First Router Capability TLV.
+                * Allocate router cap structure and initialize SR Algorithms */
+               tlvs->router_cap = XCALLOC(MTYPE_ISIS_TLV,
+                                          sizeof(struct isis_router_cap));
                for (int i = 0; i < SR_ALGORITHM_COUNT; i++)
-                       rcap->algo[i] = SR_ALGORITHM_UNSET;
+                       tlvs->router_cap->algo[i] = SR_ALGORITHM_UNSET;
        }
 
+       rcap = tlvs->router_cap;
+
        /* Get Router ID and Flags */
        rcap->router_id.s_addr = stream_get_ipv4(s);
        rcap->flags = stream_getc(s);
@@ -5301,7 +5302,6 @@ static int unpack_tlv_router_cap(enum isis_tlv_context context,
                                log, indent,
                                "WARNING: Router Capability subTLV length too large compared to expected size\n");
                        stream_forward_getp(s, STREAM_READABLE(s));
-                       XFREE(MTYPE_ISIS_TLV, rcap);
                        return 0;
                }
 
@@ -5612,7 +5612,6 @@ static int unpack_tlv_router_cap(enum isis_tlv_context context,
                }
                subtlv_len = subtlv_len - length - 2;
        }
-       tlvs->router_cap = rcap;
        return 0;
 }