code-423n4 / 2023-07-axelar-findings

2 stars 0 forks source link

InterchainProposalExecutor doesn't support actions with value #476

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-07-axelar/blob/main/contracts/interchain-governance-executor/InterchainProposalExecutor.sol#L76

Vulnerability details

Impact

An interchain call consists of the target address, calldata, and value. When InterchainProposalExecutor performs the call, it passes the value along

    function _executeProposal(InterchainCalls.Call[] memory calls) internal {
        for (uint256 i = 0; i < calls.length; i++) {
            InterchainCalls.Call memory call = calls[i];
            (bool success, bytes memory result) = call.target.call{ value: call.value }(call.callData);

This call will fail for non-zero value because this code path is not payable. In fact, there are no payable or receive methods neither in InterchainProposalExecutor, nor in any of its parent contracts. The only way this may not fail is if someone force feeds the contract via selfdestruct. It is safe to assume this is not the intended way to use the contract.

Proof of Concept

https://github.com/code-423n4/2023-07-axelar/blob/main/contracts/interchain-governance-executor/InterchainProposalExecutor.sol#L73-L76

Tools Used

Manual

Recommended Mitigation Steps

I think the best option is to implement it similar to how InterchainGovernance work. After the message is approved by AxelarGateway, save it in storage and allow anyone to execute it in a separate call, while attaching the value along.

Another option would be to add a receive method so that the executor's balance can be refilled before executing an action.

Assessed type

call/delegatecall

c4-pre-sort commented 1 year ago

0xSorryNotSorry marked the issue as duplicate of #319

c4-judge commented 1 year ago

berndartmueller marked the issue as satisfactory