Skip to content

Commit e3626a4

Browse files
authored
[MSTP] Fixing buffer OOB Read/Write issues (#72)
* Fixing buffer OOB Read/Write issues * Corrected indentation
1 parent 94908be commit e3626a4

File tree

1 file changed

+33
-3
lines changed

1 file changed

+33
-3
lines changed

mstp/mstp_util.c

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -667,6 +667,7 @@ void mstputil_network_order_bpdu(MSTP_BPDU *bpdu)
667667
{
668668
#if __BYTE_ORDER == __LITTLE_ENDIAN
669669
UINT16 i;
670+
UINT16 num_msti_messages;
670671

671672
bpdu->protocol_id = htons(bpdu->protocol_id);
672673
if (bpdu->type == TCN_BPDU_TYPE)
@@ -688,7 +689,14 @@ void mstputil_network_order_bpdu(MSTP_BPDU *bpdu)
688689
bpdu->protocol_version_id == STP_VERSION_ID)
689690
return;
690691

691-
for (i = 0; i < MSTP_GET_NUM_MSTI_CONFIG_MESSAGES(bpdu->v3_length); i++)
692+
num_msti_messages = MSTP_GET_NUM_MSTI_CONFIG_MESSAGES(bpdu->v3_length);
693+
694+
// Validate the number of MSTI messages to prevent buffer overflow
695+
if (num_msti_messages > MSTP_MAX_INSTANCES_PER_REGION) {
696+
num_msti_messages = MSTP_MAX_INSTANCES_PER_REGION;
697+
}
698+
699+
for (i = 0; i < num_msti_messages; i++)
692700
{
693701
HOST_TO_NET_MAC(&bpdu->msti_msgs[i].msti_regional_root.address,
694702
&bpdu->msti_msgs[i].msti_regional_root.address);
@@ -713,6 +721,7 @@ void mstputil_host_order_bpdu(MSTP_BPDU *bpdu)
713721
{
714722
#if __BYTE_ORDER == __LITTLE_ENDIAN
715723
UINT16 i;
724+
UINT16 num_msti_messages;
716725

717726
bpdu->protocol_id = ntohs(bpdu->protocol_id);
718727
if (bpdu->type == TCN_BPDU_TYPE)
@@ -734,7 +743,14 @@ void mstputil_host_order_bpdu(MSTP_BPDU *bpdu)
734743
return;
735744

736745
bpdu->v3_length = ntohs(bpdu->v3_length);
737-
for (i = 0; i < MSTP_GET_NUM_MSTI_CONFIG_MESSAGES(bpdu->v3_length); i++)
746+
num_msti_messages = MSTP_GET_NUM_MSTI_CONFIG_MESSAGES(bpdu->v3_length);
747+
748+
// Validate the number of MSTI messages to prevent buffer overflow
749+
if (num_msti_messages > MSTP_MAX_INSTANCES_PER_REGION) {
750+
num_msti_messages = MSTP_MAX_INSTANCES_PER_REGION;
751+
}
752+
753+
for (i = 0; i < num_msti_messages; i++)
738754
{
739755
NET_TO_HOST_MAC(&bpdu->msti_msgs[i].msti_regional_root.address,
740756
&bpdu->msti_msgs[i].msti_regional_root.address);
@@ -863,6 +879,15 @@ bool mstputil_validate_bpdu(MSTP_BPDU *bpdu, UINT16 size)
863879
return true;
864880
}
865881

882+
// Additional validation: check if v3_length would cause buffer overflow
883+
// when calculating the number of MSTI configuration messages
884+
UINT16 num_msti_messages = MSTP_GET_NUM_MSTI_CONFIG_MESSAGES(bpdu->v3_length);
885+
if (num_msti_messages > MSTP_MAX_INSTANCES_PER_REGION) {
886+
STP_LOG_ERR("mstp bpdu v3_length %u would cause %u MSTI messages, exceeding limit of %u",
887+
bpdu->v3_length, num_msti_messages, MSTP_MAX_INSTANCES_PER_REGION);
888+
return false;
889+
}
890+
866891
// mstp bpdu
867892
return true;
868893
}
@@ -1407,6 +1432,11 @@ static UINT16 mstputil_get_bpdu_size(MSTP_BPDU *bpdu)
14071432
msti_config_msgsize = sizeof(MSTI_CONFIG_MESSAGE);
14081433
num_instances = MSTP_GET_NUM_MSTI_CONFIG_MESSAGES(v3_length);
14091434

1435+
// Validate the number of MSTI messages to prevent potential underflow
1436+
if (num_instances > max_instances) {
1437+
num_instances = max_instances;
1438+
}
1439+
14101440
bpdu_size -= ((max_instances-num_instances) * msti_config_msgsize);
14111441
}
14121442

@@ -2242,4 +2272,4 @@ void mstputil_update_mst_stats(PORT_ID port_number, MSTP_MSTI_PORT *msti_port, b
22422272
msti_port->co.stats.rx_bpdu++;
22432273
else
22442274
msti_port->co.stats.tx_bpdu++;
2245-
}
2275+
}

0 commit comments

Comments
 (0)