sherlock-audit / 2023-12-avail-judging

4 stars 4 forks source link

ravikiran.web3 - Messages flowing from Ethereum to Avail Blockchain does not have a message consumed lock #41

Closed sherlock-admin closed 8 months ago

sherlock-admin commented 8 months ago

ravikiran.web3

medium

Messages flowing from Ethereum to Avail Blockchain does not have a message consumed lock

Summary

While the messages flowing from Avail to Ethereum have a spent lock, which means the message is consumed only once as there is a mechanism to track the message. There is no similar tracking available for messages flowing from Ethereum to Avail Blockchain. The light weight client does not have support from bridge contract to track and prevent double spending.

Vulnerability Detail

While it is possible to queue exact same instruction a few times, potentially due to occurrence, a message queued for execution in Avail is not tracked for its status as pending or processed.

Refer to the below functions: a) sendMessage() b) sendAVAIL() c) sendETH()

As such, there is a possibility to process the same exact message in the queue twice. Currently, the reliance is on light weight client to process these messages only once.

Impact

Double spend problem

Code Snippet

sendMessage() https://github.com/sherlock-audit/2023-12-avail/blob/main/contracts/src/AvailBridge.sol#L300-L321

sendAVAIL() https://github.com/sherlock-audit/2023-12-avail/blob/main/contracts/src/AvailBridge.sol#L329-L349

sendETH() https://github.com/sherlock-audit/2023-12-avail/blob/main/contracts/src/AvailBridge.sol#L356-L374

sendERC20() function https://github.com/sherlock-audit/2023-12-avail/blob/main/contracts/src/AvailBridge.sol#L383-L411

In all the above functions, the message is queued, but there is no provision to mark the status of processing.

        // store message hash to be retrieved later by our light client
        isSent[id] = keccak256(abi.encode(message));

Tool used

Manual Review

Recommendation

Add tracking for each queued message and track the status.

To do that, create a struct as below and track it.

struct ReceivedMsg{
  bytes32 msg;
  bool isProcessed;
}

  mapping(uint256 => ReceivedMsg) public isSent;

and then modify each of the receive function to store the message as below.

  isSent[id] = ReceivedMsg(keccak256(abi.encode(message)), false); 

Also provision a function to mark it as process and restrict the marking to only avail light weight client.

sherlock-admin commented 8 months ago

1 comment(s) were left on this issue during the judging contest.

takarez commented:

invalid because {invalid: no reasonable impact}