Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
c4-bot-8 opened this issue Jul 2, 2024 · 6 comments
Closed

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

c4-bot-8 opened this issue Jul 2, 2024 · 6 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-238 🤖_48_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") sufficient quality report This report is of sufficient quality

Comments

@c4-bot-8
Copy link
Contributor

c4-bot-8 commented Jul 2, 2024

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

@c4-bot-8 c4-bot-8 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Jul 2, 2024
c4-bot-8 added a commit that referenced this issue Jul 2, 2024
@c4-bot-12 c4-bot-12 added the 🤖_48_group AI based duplicate group recommendation label Jul 2, 2024
@howlbot-integration howlbot-integration bot added primary issue Highest quality submission among a set of duplicates sufficient quality report This report is of sufficient quality labels Jul 4, 2024
@aviggiano aviggiano added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label Jul 4, 2024
@aviggiano
Copy link

Fixed in SizeCredit/size-solidity#126

@c4-judge
Copy link
Contributor

hansfriese marked the issue as satisfactory

@c4-judge c4-judge added satisfactory satisfies C4 submission criteria; eligible for awards selected for report This submission will be included/highlighted in the audit report labels Jul 10, 2024
@c4-judge
Copy link
Contributor

hansfriese marked the issue as selected for report

This was referenced Jul 10, 2024
@c4-judge
Copy link
Contributor

hansfriese marked the issue as not selected for report

@c4-judge c4-judge removed the selected for report This submission will be included/highlighted in the audit report label Jul 21, 2024
@c4-judge
Copy link
Contributor

hansfriese marked the issue as duplicate of #238

@c4-judge c4-judge added duplicate-238 and removed primary issue Highest quality submission among a set of duplicates labels Jul 21, 2024
@c4-judge
Copy link
Contributor

hansfriese marked the issue as duplicate of #238

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working duplicate-238 🤖_48_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

4 participants