Out of bounds multicast group ID can cause memory corruption

[gregbreen]

In handling several of the multicast package commands, the code uses the group ID directly from the incoming payload, and indexes into McSessionData array without range checking the ID value. For example:

            case REMOTE_MCAST_SETUP_MC_GROUP_SETUP_REQ:
            {
                uint8_t id = mcpsIndication->Buffer[cmdIndex++];
                McSessionData[id].McGroupData.IdHeader.Value = id;

                McSessionData[id].McGroupData.McAddr =  ( mcpsIndication->Buffer[cmdIndex++] << 0  ) & 0x000000FF;
                McSessionData[id].McGroupData.McAddr += ( mcpsIndication->Buffer[cmdIndex++] << 8  ) & 0x0000FF00;
                McSessionData[id].McGroupData.McAddr += ( mcpsIndication->Buffer[cmdIndex++] << 16 ) & 0x00FF0000;
                McSessionData[id].McGroupData.McAddr += ( mcpsIndication->Buffer[cmdIndex++] << 24 ) & 0xFF000000;

This is fine if LORAMAC_MAX_MC_CTX is 4. (Although in the above example, id could be as much as 255.) However if LORAMAC_MAX_MC_CTX is changed to a lower value (such as 1 in the STM32CubeWL firmware), this can be more problematic. The out of bounds array write occurs before an IDerror is determined.

If you agree, then REMOTE_MCAST_SETUP_MC_GROUP_CLASS_C_SESSION_REQ and REMOTE_MCAST_SETUP_MC_GROUP_CLASS_B_SESSION_REQ have a similar vulnerability. They are both ANDed with 0x03, but that has the assumption that LORAMAC_MAX_MC_CTX is 4.

[MarekNovakACRIOS]

I have met a similar problem here - #1277

[mluis1]

Thanks for reporting this issue.
We will try to fix it for next release. Or maybe you could propose a PR to fix it.

[mluis1]

@gregbreen and @MarekNovakACRIOS Would it be possible for your to test the below attached patch.

I think it should fix the observed issue. However I did not had the time yet to verify it.

Your feedback are welcome.

Thanks in advance.

mc_setup_out_of_bounds_check.patch.txt

[gregbreen]

Thanks @mluis1 . Since I'm using the STM32CubeWL firmware, this patch doesn't easily apply for me. I think the changes you made look right.

[mluis1]

Thanks for the feedback.

We will push the patch as soon as possible.