code-423n4 / 2023-06-ambire-mitigation-findings

0 stars 0 forks source link

M-02 MitigationConfirmed #13

Open code423n4 opened 1 year ago

code423n4 commented 1 year ago

Lines of code

Vulnerability details

Issue

M-02: Attacker can force the failure of transactions that use tryCatch

Mitigation

https://github.com/AmbireTech/ambire-common/commit/fca50cd45478ef1ab57ba21bf7e1ccd15b310a05

Assessment of Mitigation

Issue mitigated

Comments

This mitigation updates the following AmbireAccount.tryCatch function to execute (bool success, bytes memory returnData) = to.call{ value: value, gas: gasBefore }(data) after uint256 gasBefore = gasleft() and before require(gasleft() > gasBefore/64, 'TRYCATCH_OOG'). Since the upper gas limit for the inner to.call function call is set to gasBefore that is the gas left in the AmbireAccount.tryCatch function's context just before this to.call function call, if this to.call function call consumes more than 63/64 of gasBefore, then there is no way that gasleft() > gasBefore/64 can be true after such to.call function call; in this case, the gas consumption of more than 63/64 of gasBefore is too much comparing to all but one 64th of the maximum allowed amount of gas that would be given to this to.call function if it could be called without inputting gas, which is specified by EIP-150. In order to not revert require(gasleft() > gasBefore/64, 'TRYCATCH_OOG'), enough gas needs to be provided to the inner to.call function so such to.call function call would not revert due to out of gas. As a result, this mitigation makes the malicious relayer's or actor's manipulation of the gas for calling the AmbireAccount.tryCatch function ineffective and prevents such relayer or actor from forcing the inner to.call function call to revert due to out of gas. Therefore, the corresponding issue is mitigated.

https://github.com/AmbireTech/ambire-common/blob/f56e7dcaaa4ff8950b39fe6d5bced165d0f1c99f/contracts/AmbireAccount.sol#L113-L119

    function tryCatch(address to, uint256 value, bytes calldata data) external payable {
        require(msg.sender == address(this), 'ONLY_IDENTITY_CAN_CALL');
        uint256 gasBefore = gasleft();
        (bool success, bytes memory returnData) = to.call{ value: value, gas: gasBefore }(data);
        require(gasleft() > gasBefore/64, 'TRYCATCH_OOG');
        if (!success) emit LogErr(to, value, data, returnData);
    }
c4-judge commented 1 year ago

Picodes marked the issue as satisfactory