Messages[] in MessageProxyforMainNet & SChain won't be cleared

[code423n4]

Lines of code

https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/MessageProxyForMainnet.sol#L208 https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/schain/MessageProxyForSchain.sol#L203

Vulnerability details

Since the IMA depends on messages (to transfer the different tokens⇒ERC20, ERC1155, ERC721 et al or other arbitrary data ) being relayed between Mainnet and the other skale chains(and among themselves), it's understandable that there is a need to limit the amount of incoming messages but there's no way to send messages after the maximum amount of msgs has been reached.

In the line below, the array will still have the previous messages and will not be emptied :

https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/MessageProxyForMainnet.sol#L208 https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/schain/MessageProxyForSchain.sol#L203

So, the function will be rendered useless. Either cache the length after it is filled and then shift the items (move older message and shift (-1)newer ones to the start of the array and pop the older value from the end.

[cstrangedk]

There are no limits for messages sent through messageProxy, there is only a limit to the batch size of message. ** Messages are transacted through a messaging queue for each connected chain https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/MessageProxy.sol#L43

Agent can send messages in batches, there is a limit to the batch of batch of messages. The agent is limited to send 4 messages, in the MessageProxy batch limit is 10 messages.

[DimaStebaev]

There is no issue.

A source chain emits messages. Each of them has implicit serial number. Offchain agents parse them create a batch, sign it and send to postIncomingMessages function as messages parameter. It's very important that startingCounter is passed. It's a serial number of the first message in the batch. During a batch signing agents look at amount of processed messages and do not sign one if it contains already processed messages or there are any gaps. startingCounter must always be equal to amount of already processed messages. Look at these lines: https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/MessageProxyForMainnet.sol#L220 https://github.com/skalenetwork/ima-c4-audit/blob/11d6a6ae5bf16af552edd75183791375e501915f/contracts/mainnet/MessageProxyForMainnet.sol#L251

require(
    startingCounter == connectedChains[fromSchainHash].incomingMessageCounter,
    "Starting counter is not equal to incoming message counter"
);
...
connectedChains[fromSchainHash].incomingMessageCounter += messages.length;

There is no useless execution because already sent messages are not sent again and arbitrary big amount of messages can be sent eventually but not in the single transaction because of batch size limit.

[GalloDaSballo]

Given the code in scope I agree with the sponsor that there's no vulnerability here.

Arguably the Validators could sign and execute the same messages multiple times, however this is beyond the scope of this contest