From: Juraj Vijtiuk Date: Wed, 13 Oct 2021 16:32:53 +0000 (+0200) Subject: isisd: fix router capability TLV parsing issues X-Git-Tag: pim6-testing-20220430~355^2 X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=9ba865f54d331c550629304cb25e77ac81455803;p=mirror%2Ffrr.git isisd: fix router capability TLV parsing issues isis_tlvs.c would fail at multiple places if incorrect TLVs were received causing stream assertion violations. This patch fixes the issues by adding missing length checks, missing consumed length updates and handling malformed Segment Routing subTLVs. Signed-off-by: Juraj Vijtiuk Small adjustments by Igor Ryzhov: - fix incorrect replacement of srgb by srlb on lines 3052 and 3054 - add length check for ISIS_SUBTLV_ALGORITHM - fix conflict in fuzzing data during rebase Signed-off-by: Igor Ryzhov --- diff --git a/isisd/isis_tlvs.c b/isisd/isis_tlvs.c index 9a442e0374..f1aae7caf1 100644 --- a/isisd/isis_tlvs.c +++ b/isisd/isis_tlvs.c @@ -3007,28 +3007,55 @@ static int unpack_tlv_router_cap(enum isis_tlv_context context, type = stream_getc(s); length = stream_getc(s); + + if (length > STREAM_READABLE(s) || length > subtlv_len - 2) { + sbuf_push( + log, indent, + "WARNING: Router Capability subTLV length too large compared to expected size\n"); + stream_forward_getp(s, STREAM_READABLE(s)); + + return 0; + } + switch (type) { case ISIS_SUBTLV_SID_LABEL_RANGE: /* Check that SRGB is correctly formated */ if (length < SUBTLV_RANGE_LABEL_SIZE || length > SUBTLV_RANGE_INDEX_SIZE) { stream_forward_getp(s, length); - continue; + break; } /* Only one SRGB is supported. Skip subsequent one */ if (rcap->srgb.range_size != 0) { stream_forward_getp(s, length); - continue; + break; } rcap->srgb.flags = stream_getc(s); rcap->srgb.range_size = stream_get3(s); /* Skip Type and get Length of SID Label */ stream_getc(s); size = stream_getc(s); - if (size == ISIS_SUBTLV_SID_LABEL_SIZE) + + if (size == ISIS_SUBTLV_SID_LABEL_SIZE + && length != SUBTLV_RANGE_LABEL_SIZE) { + stream_forward_getp(s, length - 6); + break; + } + + if (size == ISIS_SUBTLV_SID_INDEX_SIZE + && length != SUBTLV_RANGE_INDEX_SIZE) { + stream_forward_getp(s, length - 6); + break; + } + + if (size == ISIS_SUBTLV_SID_LABEL_SIZE) { rcap->srgb.lower_bound = stream_get3(s); - else + } else if (size == ISIS_SUBTLV_SID_INDEX_SIZE) { rcap->srgb.lower_bound = stream_getl(s); + } else { + stream_forward_getp(s, length - 6); + break; + } /* SRGB sanity checks. */ if (rcap->srgb.range_size == 0 @@ -3042,9 +3069,12 @@ static int unpack_tlv_router_cap(enum isis_tlv_context context, /* Only one range is supported. Skip subsequent one */ size = length - (size + SUBTLV_SR_BLOCK_SIZE); if (size > 0) - stream_forward_getp(s, length); + stream_forward_getp(s, size); + break; case ISIS_SUBTLV_ALGORITHM: + if (length == 0) + break; /* Only 2 algorithms are supported: SPF & Strict SPF */ stream_get(&rcap->algo, s, length > SR_ALGORITHM_COUNT @@ -3059,12 +3089,12 @@ static int unpack_tlv_router_cap(enum isis_tlv_context context, if (length < SUBTLV_RANGE_LABEL_SIZE || length > SUBTLV_RANGE_INDEX_SIZE) { stream_forward_getp(s, length); - continue; + break; } /* RFC 8667 section #3.3: Only one SRLB is authorized */ if (rcap->srlb.range_size != 0) { stream_forward_getp(s, length); - continue; + break; } /* Ignore Flags which are not defined */ stream_getc(s); @@ -3072,10 +3102,27 @@ static int unpack_tlv_router_cap(enum isis_tlv_context context, /* Skip Type and get Length of SID Label */ stream_getc(s); size = stream_getc(s); - if (size == ISIS_SUBTLV_SID_LABEL_SIZE) + + if (size == ISIS_SUBTLV_SID_LABEL_SIZE + && length != SUBTLV_RANGE_LABEL_SIZE) { + stream_forward_getp(s, length - 6); + break; + } + + if (size == ISIS_SUBTLV_SID_INDEX_SIZE + && length != SUBTLV_RANGE_INDEX_SIZE) { + stream_forward_getp(s, length - 6); + break; + } + + if (size == ISIS_SUBTLV_SID_LABEL_SIZE) { rcap->srlb.lower_bound = stream_get3(s); - else + } else if (size == ISIS_SUBTLV_SID_INDEX_SIZE) { rcap->srlb.lower_bound = stream_getl(s); + } else { + stream_forward_getp(s, length - 6); + break; + } /* SRLB sanity checks. */ if (rcap->srlb.range_size == 0 @@ -3089,13 +3136,14 @@ static int unpack_tlv_router_cap(enum isis_tlv_context context, /* Only one range is supported. Skip subsequent one */ size = length - (size + SUBTLV_SR_BLOCK_SIZE); if (size > 0) - stream_forward_getp(s, length); + stream_forward_getp(s, size); + break; case ISIS_SUBTLV_NODE_MSD: /* Check that MSD is correctly formated */ if (length < MSD_TLV_SIZE) { stream_forward_getp(s, length); - continue; + break; } msd_type = stream_getc(s); rcap->msd = stream_getc(s); diff --git a/isisd/isis_tlvs.h b/isisd/isis_tlvs.h index 38470ef85e..0c6ed11cb6 100644 --- a/isisd/isis_tlvs.h +++ b/isisd/isis_tlvs.h @@ -447,6 +447,7 @@ enum ext_subtlv_size { /* RFC 8667 sections #2 & #3 */ ISIS_SUBTLV_SID_LABEL_SIZE = 3, + ISIS_SUBTLV_SID_INDEX_SIZE = 4, ISIS_SUBTLV_SID_LABEL_RANGE_SIZE = 9, ISIS_SUBTLV_ALGORITHM_SIZE = 4, ISIS_SUBTLV_ADJ_SID_SIZE = 5, diff --git a/tests/isisd/test_fuzz_isis_tlv_tests.h.gz b/tests/isisd/test_fuzz_isis_tlv_tests.h.gz index accc906bf2..20b1dc33f9 100644 Binary files a/tests/isisd/test_fuzz_isis_tlv_tests.h.gz and b/tests/isisd/test_fuzz_isis_tlv_tests.h.gz differ