remoteCall does not revert if dispatchMessage failed for all of the AMBs

[rmi7]

Severity

Medium

Difficulty

High

Type

Undefined Behavior

Target

src/MultiBridgeMessageSender.sol

Description

If the dispatchMessage call fails for every Arbitrary Message Bridge (AMB), than the remoteCall transaction does not revert. This will result in an approved DAO proposal execution to not be executed. To re-execute the proposal it will have to be proposed again, voted upon again, scheduled in the GovernanceTimelock contract again, and waited upon until the delay has passed, before it can be re-executed again (which might fail, again).

/// @dev if one bridge is paused, the flow shouldn't be broken
try IMessageSenderAdapter(_adapters[i]).dispatchMessage{value: _fees[i]}(
    _dstChainId, _mmaReceiver, abi.encode(_message)
) {
    successes[i] = true;
} catch {
    successes[i] = false;
    emit MessageSendFailed(_adapters[i], _message);
}

Source: src/MultiBridgeMessageSender.sol#L364-L371

A DAO proposal that has gathered enough votes will be scheduled in the GovernanceTimelock contract. Once the delay time has passed the GovernanceTimelock.executeTransaction function can be called, which will call the MultiBridgeMessageSender.remoteCall function. This will attempt to dispatch the message through every configured AMB. The implementation keeps track of which AMB dispatches were successful. This information is emitted in the MultiBridgeMessageSent event at the end of remoteCall. Which might actually indicate that no messages were sent if all of the successes values are false.

There is a comment that explains that if one bridge is paused, the flow shouldn't be broken. However, if all of the AMBs fail than it makes sense to simply revert the transaction. This way the proposal that should be executed, can be retried at a later time without having to re-propose the proposal.

Exploit Scenario

Bob executes a scheduled proposal whose delay has passed in the GovernanceTimelock contract. Of the two configured AMBs, both fail. The transaction succeeds and the emitted MultiBridgeMessageSent event indicates that the message was not send to any of the AMBs, effectively skipping the execution of this DAO proposal.

Recommendations

Short term, revert the transaction if a crosschain message was failed to be dispatched to all(!) of the AMBs.

Long term, think about how the failing of bridges can affect the execution of successfully passed DAO proposals. Should failing bridges prevent a proposal from being executed (by continuing the transaction and only logging the failure of a bridge dispatch in an event) or should they not (by being able to retry the execution at a later time)? Also take into consideration the quorum and how a failing bridge might mean the quorum can never be reached. Adding the possibility of retrying the execution of proposals seems to be the way forward.

[ermyas]

Fixed through #97