Returned Value Of Function Call Not Verified

[code423n4]

Lines of code

https://github.com/code-423n4/2022-07-axelar/blob/3729dd4aeff8dc2b8b9c3670a1c792c81fc60e7c/contracts/AxelarGateway.sol#L320

Vulnerability details

When executing a function call, two data are returned (success condition and return data).

It was observed that after executing the command via a function call, the contract verifies the success condition to ensure that the call did not revert. However, it does not verify the return data of the call. It might be possible that the function call has failed, but it does not revert and return a false boolean instead. In this case, a failed execution will be wrongly marked as having been successfully executed in the system, and it cannot be re-executed again.

https://github.com/code-423n4/2022-07-axelar/blob/3729dd4aeff8dc2b8b9c3670a1c792c81fc60e7c/contracts/AxelarGateway.sol#L320

function execute(bytes calldata input) external override {
    ..SNIP..

        // Prevent a re-entrancy from executing this command before it can be marked as successful.
        _setCommandExecuted(commandId, true);
        (bool success, ) = address(this).call(abi.encodeWithSelector(commandSelector, params[i], commandId));

        if (success) emit Executed(commandId);
        else _setCommandExecuted(commandId, false);
    }
}

Recommendation

Verify both success condition and return data returned by the function call to ensure that the call did not fail.

function execute(bytes calldata input) external override {
    ..SNIP..

        // Prevent a re-entrancy from executing this command before it can be marked as successful.
        _setCommandExecuted(commandId, true);
-       (bool success, ) = address(this).call(abi.encodeWithSelector(commandSelector, params[i], commandId));
+       (bool success, bytes memory returnData) = address(this).call(abi.encodeWithSelector(commandSelector, params[i], commandId));

+       bool executedSuccess = success && (returnData.length == uint256(0) || abi.decode(returnData, (bool)))
+       if (executedSuccess) emit Executed(commandId);
-       if (success) emit Executed(commandId);
        else _setCommandExecuted(commandId, false);
    }
}

[re1ro]

Not applicable. Those are calls to our own gateway functions and they are not returning anything. They revert if things go wrong

[GalloDaSballo]

We can verify here: https://github.com/code-423n4/2022-07-axelar/blob/3729dd4aeff8dc2b8b9c3670a1c792c81fc60e7c/contracts/AxelarGateway.sol#L300-L313

That all those selectors map out to function with a 0 return value, contract existence is guaranteed by calling self

For those reasons, I agree with the sponsor