code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-yieldy/blob/524f3b83522125fb7d4677fa7a7e5ba5a2c0fe67/src/contracts/LiquidityReserve.sol#L100-L127

Vulnerability details

Impact

Function addLiquidity() suppose to do add Liquidity for the staking Token and receive lrToken in exchange. to calculate amount of IrToken codes uses this calculation: amountToMint = (_amount * lrFoxSupply) / totalLockedValue but it's possible for attacker to manipulate totalLockedValue (by sending tokens directly to LiquidityReserve address) and make totalLockedValue/lrFoxSupply very high in early stage of contract deployment so because of rounding error in calculation of amountToMint the users would receive very lower IrToken and users funds would be lost and attacker can steal them. Attacker can perform this attack by sending tokens before even LiquidityReserve deployed because the contract address would be predictable and attacker can perform front-run or sandwich attack too.

also it's possible to perform this attack for STAKING_TOKEN with low precision and low price even if LiquidityReserve had some balances.

Proof of Concept

This is addLiquidity() code in LiquidityReserve:

    function addLiquidity(uint256 _amount) external {
        require(isReserveEnabled, "Not enabled yet");
        uint256 stakingTokenBalance = IERC20Upgradeable(stakingToken).balanceOf(
            address(this)
        );
        uint256 rewardTokenBalance = IERC20Upgradeable(rewardToken).balanceOf(
            address(this)
        );
        uint256 lrFoxSupply = totalSupply();
        uint256 coolDownAmount = IStaking(stakingContract)
            .coolDownInfo(address(this))
            .amount;
        uint256 totalLockedValue = stakingTokenBalance +
            rewardTokenBalance +
            coolDownAmount;

        uint256 amountToMint = (_amount * lrFoxSupply) / totalLockedValue;
        IERC20Upgradeable(stakingToken).safeTransferFrom(
            msg.sender,
            address(this),
            _amount
        );
        _mint(msg.sender, amountToMint);
    }

As you can see code uses this calculation: amountToMint = (_amount * lrFoxSupply) / totalLockedValue; to find the amount of IrToken that is going to mint for user. but attacker can send stakingToken or rewardToken directly to LiquidityReserve address when the there is no liqudity in the contract and make totalLockedValue very high. then attacker call addLiquidity() and mint some IrToken for himself and from then anyone tries to call addLiquidity() because of rounding error is going to lose some funds (receives less IrToken than he is supposed to)

Tools Used

VIM

Recommended Mitigation Steps

add more precision when calculating IrToken so this attack wouldn't be feasible to perform.

toshiSat commented 2 years ago

sponsor confirmed

0xean commented 2 years ago

ahh the inflation attack..... Wardens love this one.

The contract locks a minimum liquidity amount which blocks the feasibility attack for the most part. Please see enableLiquidityReserve for the code where the locking occurs.

moose-code commented 2 years ago

Some good worthwhile ideas from the warden but after reviewing the enableLiquidityReserve going to downgrade this to medium. After reading the code and the described attack, its not very clear how the attacker would benefit and bring the contract into this state.

By sending tokens directly to the contract (expensive) and increasing total totalLockedValue, this will decrease the amount the amountToMint for the user but unclear that this cost is worth it or how an attacker could actually benefit (from what I can see).

Think its still worth exploring this vector in more depth as its a creative attack. Warrants medium and further investigation.

code-423n4 / 2022-06-yieldy-findings