Skip to content

Commit

Permalink
isisd: fix rcap tlv double-free crash
Browse files Browse the repository at this point in the history
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 78774bb ("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: 49efc80 ("isisd: Ensure rcap is freed in error case")
Fixes: 78774bb ("isisd: add isis flex-algo lsp advertisement")
Reported-by: Iggy Frankovic <[email protected]>
Signed-off-by: Louis Scalbert <[email protected]>
(cherry picked from commit d617581)
  • Loading branch information
louis-6wind authored and mergify[bot] committed Sep 17, 2024
1 parent 55aa5a3 commit 31d39e7
Showing 1 changed file with 8 additions and 9 deletions.
17 changes: 8 additions & 9 deletions isisd/isis_tlvs.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down

0 comments on commit 31d39e7

Please sign in to comment.