code-423n4 / 2024-06-size-findings

3 stars 1 forks source link

The `borrowATokenCap` requirement doesn't work during a multicall. #144

Closed c4-bot-8 closed 3 months ago

c4-bot-8 commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/Multicall.sol#L29 https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/Multicall.sol#L37

Vulnerability details

Impact

Users can mint more borrowAToken than borrowATokenCap using a multicall.

Proof of Concept

During a multicall, it validates the borrowATokenCap requirement after executing functions.

    function multicall(State storage state, bytes[] calldata data) internal returns (bytes[] memory results) {
 state.data.isMulticall = true;

        uint256 borrowATokenSupplyBefore = state.data.borrowAToken.balanceOf(address(this)); //@audit wrong supply
        uint256 debtTokenSupplyBefore = state.data.debtToken.totalSupply();

 results = new bytes[](data.length);
        for (uint256 i = 0; i < data.length; i++) {
 results[i] = Address.functionDelegateCall(address(this), data[i]);
 }

        uint256 borrowATokenSupplyAfter = state.data.borrowAToken.balanceOf(address(this)); //@audit  wrong supply
        uint256 debtTokenSupplyAfter = state.data.debtToken.totalSupply();

 state.validateBorrowATokenIncreaseLteDebtTokenDecrease(
 borrowATokenSupplyBefore, debtTokenSupplyBefore, borrowATokenSupplyAfter, debtTokenSupplyAfter
 );

 state.data.isMulticall = false;
 }

But it uses borrowAToken.balanceOf(address(this)) instead of borrowAToken.totalSupply() and it won't work as expected because borrowAToken.balanceOf(address(this)) < borrowAToken.totalSupply().

As a result, validateBorrowATokenIncreaseLteDebtTokenDecrease() would pass when it should revert with lower borrowATokenSupplyBefore/borrowATokenSupplyAfter amounts.

Tools Used

Manual Review

Recommended Mitigation Steps

It should validate using borrowAToken.totalSupply().

 function multicall(State storage state, bytes[] calldata data) internal returns (bytes[] memory results) {
 state.data.isMulticall = true;

-        uint256 borrowATokenSupplyBefore = state.data.borrowAToken.balanceOf(address(this));
+        uint256 borrowATokenSupplyBefore = state.data.borrowAToken.totalSupply();
 uint256 debtTokenSupplyBefore = state.data.debtToken.totalSupply();

 results = new bytes[](data.length);
 for (uint256 i = 0; i < data.length; i++) {
 results[i] = Address.functionDelegateCall(address(this), data[i]);
 }

-        uint256 borrowATokenSupplyAfter = state.data.borrowAToken.balanceOf(address(this));
+        uint256 borrowATokenSupplyAfter = state.data.borrowAToken.totalSupply();
 uint256 debtTokenSupplyAfter = state.data.debtToken.totalSupply();

 state.validateBorrowATokenIncreaseLteDebtTokenDecrease(
 borrowATokenSupplyBefore, debtTokenSupplyBefore, borrowATokenSupplyAfter, debtTokenSupplyAfter
 );

 state.data.isMulticall = false;
 }

Assessed type

Invalid Validation

aviggiano commented 4 months ago

Fixed in https://github.com/SizeCredit/size-solidity/pull/126

c4-judge commented 3 months ago

hansfriese marked the issue as satisfactory

c4-judge commented 3 months ago

hansfriese marked the issue as selected for report

c4-judge commented 3 months ago

hansfriese marked the issue as not selected for report

c4-judge commented 3 months ago

hansfriese marked the issue as duplicate of #238

c4-judge commented 3 months ago

hansfriese marked the issue as duplicate of #238