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 <juraj.vijtiuk@sartura.hr>
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 <iryzhov@nfware.com>

diff --git a/isisd/isis_tlvs.c b/isisd/isis_tlvs.c
index 9a442e0374888c7005fd3e3a356ea01dbf66a628..f1aae7caf1074895ab408bf8cdfab0d28bcb5d32 100644 (file)
--- 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 38470ef85ecbeb1c8b2f213602f49d3121acd9b2..0c6ed11cb63e5196ecdd74410e1a50542374c77c 100644 (file)
--- 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 accc906bf25853bd417cff25840b233f98d1221e..20b1dc33f9593661b8310dc0c205e68d022de480 100644 (file)
Binary files a/tests/isisd/test_fuzz_isis_tlv_tests.h.gz and b/tests/isisd/test_fuzz_isis_tlv_tests.h.gz differ