sherlock-audit / 2023-10-real-wagmi-judging

16 stars 14 forks source link

0xReiAyanami - DoS of protocol - borrow function will revert if contract holds holdToken #180

Closed sherlock-admin2 closed 1 year ago

sherlock-admin2 commented 1 year ago

0xReiAyanami

medium

DoS of protocol - borrow function will revert if contract holds holdToken

Summary

The LiquidityBorrowManager contract´s borrow can easily be DOSed by an attacker with a small amount of Tokens. Therefore nobody can use the protocol.

Vulnerability Detail

When borrowing Liquidity using the wagmi-leverage protocol, the borrow amount is calculated by getting the single sided value of the Liquidity, represented in the desired holdToken. Then the liquidity is extracted and all the saleTokens are swapped to the holdTokens.

The resulting amount of holdTokens is usually a bit lower than the calculated borrowAmount. This difference has to be paid by the borrower. (as the sponsor stated: a) the difference between the number of tokens received during liquidity extraction and the amount necessary to restore this liquidity in the future in case of any worst price movement.)

To achieve this there is the following code inside the borrow function.

 uint256 borrowingCollateral = cache.borrowedAmount - cache.holdTokenBalance;
(borrowingCollateral > params.maxCollateral).revertError(
    ErrLib.ErrorCode.TOO_BIG_COLLATERAL
);

// Transfer the required tokens to the VAULT_ADDRESS for collateral and holdTokenBalance
_pay(
    params.holdToken,
    msg.sender,
    VAULT_ADDRESS,
    borrowingCollateral + liquidationBonus + cache.dailyRateCollateral + feesDebt
);

As we can see the calculation simply subtracts the amount that the contract actually holds from the calculated borrow amount.

Now, a malicious user or attacker can use this fact to make all the calls to borrow for a specific holdToken going to revert. All he has to do is to transfer a small amount of the holdToken to the contract, so that the actual amount (cache.holdTokenBalance) the contract holds would be higher than the borrowedAmount.

The overflow protection of Solidity will make all calls to the function revert.

As the LiquidityBorrowManager contract is not intended to hold any Tokens and there is no sweep function, this DoS would probably be permanent.

This can be easily reproduced in the tests:

Inside the existing Hardhat Tests in the LEFT_OUTRANGE_TOKEN_1 borrowing liquidity (long position WBTC zeroForSaleToken = false) will be successful test we simply transfer a small amount of WBTC to the contract before calling the borrow function:

        //Transfer a small amount of HoldToken (WBTC) to the contract first:
        await WBTC.transfer(borrowingManager.address, ethers.utils.parseUnits("0.0009", 8)) ;
        await borrowingManager.connect(bob).borrow(params, deadline);
    });

In this case it will make the Test fail, as the function reverts with the message: reverted with panic code 0x11 (Arithmetic operation underflowed or overflowed outside of an unchecked block)

Impact

DoS of the protocol: No one can call the borrow function for the specific holdToken

Code Snippet

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

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

Tool used

Manual Review

Recommendation

Duplicate of #86