Fix premature CAN map limit (~20 signals) caused by off-by-one bounds check aliasing send/recv maps

src/canmap.cpp

@@ -360,7 +360,7 @@ int CanMap::Remove(bool rx, uint8_t messageIdx, uint8_t itemidx)
    CANPOS *lastPosMap = 0;
    CANIDMAP *map = rx ? &canRecvMap[messageIdx] : &canSendMap[messageIdx];
 
-   if (messageIdx > MAX_MESSAGES || map->first == MAX_ITEMS) return 0;
+   if (messageIdx >= MAX_MESSAGES || map->first == MAX_ITEMS) return 0;
 
    forEachPosMap(curPos, map)
    {
@@ -487,7 +487,7 @@ const CanMap::CANPOS* CanMap::GetMap(bool rx, uint8_t ididx, uint8_t itemidx, ui
 {
    CANIDMAP *map = rx ? &canRecvMap[ididx] : &canSendMap[ididx];
 
-   if (ididx > MAX_MESSAGES || map->first == MAX_ITEMS) return 0;
+   if (ididx >= MAX_MESSAGES || map->first == MAX_ITEMS) return 0;
 
    forEachPosMap(curPos, map)
    {

src/cansdo.cpp

@@ -47,7 +47,7 @@
 CanSdo::CanSdo(CanHardware* hw, CanMap* cm)
  : canHardware(hw), canMap(cm), nodeId(1), remoteNodeId(255), printRequest(-1),
    printByteIn(0), printByteOut(sizeof(printBuffer)), printTimeout(PRINT_TIMEOUT),
-   mapParam(Param::PARAM_INVALID), mapId(0), sdoReplyValid(false), sdoReplyData(0),
+   mapParam(Param::PARAM_INVALID), mapId(0xFFFFFFFF), mapInfo{}, sdoReplyValid(false), sdoReplyData(0),
    pendingUserSpaceSdo(false)
 {
    canHardware->AddCallback(this);
@@ -304,7 +304,7 @@ void CanSdo::ReadOrDeleteCanMap(SdoFrame* sdo)
 {
    bool rx = (sdo->index & 0x80) != 0;
    uint32_t canId;
-   uint8_t itemIdx = MAX(0, sdo->subIndex - 1) / 2;
+   uint8_t itemIdx = sdo->subIndex == 0 ? 0 : (sdo->subIndex - 1) / 2;
    const CanMap::CANPOS* canPos = canMap->GetMap(rx, sdo->index & 0x3f, itemIdx, canId);
 
    if (sdo->cmd == SDO_READ)

test/test_canmap.cpp

@@ -581,6 +581,36 @@ static void fail_to_map_with_invalid_big_endian_total_struct_offset()
 }
 
 
+// Regression test: GetMap and Remove must not access out-of-bounds send map entry
+// at ididx/messageIdx == MAX_MESSAGES, which would alias canRecvMap[0]
+static void get_map_at_max_messages_returns_null()
+{
+    // Add a recv mapping so canRecvMap[0] has data
+    canMap->AddRecv(Param::ocurlim, 0x300, 0, 8, 1.0, 0);
+
+    uint32_t canId = 0;
+    // Querying tx map at index MAX_MESSAGES must return null (not canRecvMap[0]'s data)
+    const CanMap::CANPOS* pos = canMap->GetMap(false, MAX_MESSAGES, 0, canId);
+    ASSERT(pos == 0);
+}
+
+// Regression test: Remove with messageIdx == MAX_MESSAGES must not delete canRecvMap[0]
+static void remove_at_max_messages_is_safe()
+{
+    // Add a recv mapping so canRecvMap[0] has data
+    canMap->AddRecv(Param::ocurlim, 0x300, 0, 8, 1.0, 0);
+
+    // Attempting to remove a tx message at index MAX_MESSAGES must be a no-op
+    int removed = canMap->Remove(false, MAX_MESSAGES, 0);
+    ASSERT(removed == 0);
+
+    // The recv mapping must still be intact
+    uint32_t canId = 0;
+    const CanMap::CANPOS* pos = canMap->GetMap(true, 0, 0, canId);
+    ASSERT(pos != 0);
+    ASSERT(canId == 0x300);
+}
+
 static void create_and_delete_complex_map_once()
 {
     canMap->AddSend(Param::amp, 257, 24, 8, -1.00, 0);
@@ -1172,4 +1202,6 @@ REGISTER_TEST(
     fail_to_map_with_invalid_big_endian_length,
     fail_to_map_with_invalid_big_endian_total_struct_offset,
     create_and_delete_complex_map_once,
+    get_map_at_max_messages_returns_null,
+    remove_at_max_messages_is_safe,
     RECEIVE_TESTS);