sherlock-audit / 2023-12-arcadia-judging

19 stars 15 forks source link

Topmark - Replay can be executed with Excess Amount during Multicall #192

Closed sherlock-admin closed 9 months ago

sherlock-admin commented 9 months ago

Topmark

medium

Replay can be executed with Excess Amount during Multicall

Summary

Replay can be executed with Excess Amount during Multicall in the Multicall.sol contract

Vulnerability Detail

function executeRepay(address creditor, address asset, address account, uint256 amount) external {
        if (amount < 1) amount = IERC20(asset).balanceOf(address(this));

        (bool success, bytes memory data) =
            creditor.call(abi.encodeWithSignature("repay(uint256,address)", amount, account));
        require(success, string(data));
    }

A careful look at the executeRepay(...) function above shows that when input amount is less than 1 i.e 0, amount is assigned the overall asset balance of the contract. This gives an idea of the limit of amount that can be used to call the executeRepay(...) function, the problem is that when amount is not zero no confirm action was done to ensure that the amount called by creditor does not go beyond the allowed threshold which means amount above the max limit can actually be used and taken advantage of

Impact

Replay can be executed with Excess Amount beyond asset balance during Multicall in the Multicall.sol contract

Code Snippet

https://github.com/sherlock-audit/2023-12-arcadia/blob/main/accounts-v2/src/actions/MultiCall.sol#L102

Tool used

Manual Review

Recommendation

As adjusted in the code below, necessary validation should be done to ensure amount done go beyond the expected limit.

function executeRepay(address creditor, address asset, address account, uint256 amount) external {
        if (amount < 1) amount = IERC20(asset).balanceOf(address(this));
+++ require ( amount <= IERC20(asset).balanceOf(address(this)) , "invalid amount") 
        (bool success, bytes memory data) =
            creditor.call(abi.encodeWithSignature("repay(uint256,address)", amount, account));
        require(success, string(data));
    }
sherlock-admin2 commented 9 months ago

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

takarez commented:

invalid: OOS

nevillehuang commented 9 months ago

Invalid, explicit checks are not required given if amount is insufficient to execute repayment it will already revert.