code-423n4 / 2024-03-revert-lend-findings

6 stars 6 forks source link

Missing slippage in deposit(), withdraw(), redeem() and mint() #497

Closed c4-bot-4 closed 3 months ago

c4-bot-4 commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/435b054f9ad2404173f36f0f74a5096c894b12b7/src/V3Vault.sol#L362-L384

Vulnerability details

When depositing, minting, withdrawing and redeeming the amount of assets returned are influenced by exchange rates.

Impact

User might experience losses when interacting with these function if the exchange rate becomes unfavorable

Proof of Concept

The functions deposit(), withdraw(), redeem() and mint() will either go through _deposit or _withdraw in order to finalize their logic, both the _deposit and _withdraw mechanism rely on volatile exchange rate that may increase or decrease depending on the current vault balances. If a user decides to withdraw or deposit at a moment where the exchange rate is lower this will negatively impact his asset.

Tools Used

Manual review

Recommended Mitigation Steps

Implement a slippage proof mechanism in order to bring more predictability to the system and its users. Consider these as inspiration:

@>   function deposit(uint256 assets, address receiver, uint256 minShares) external override returns (uint256) { 
        (, uint256 shares) = _deposit(receiver, assets, false, "");
 @>     require(shares > minShares, "slippage err");
        return shares;
    }
 @>   function mint(uint256 shares, address receiver, uint256 maxAssets) external override returns (uint256) { 
        (uint256 assets,) = _deposit(receiver, shares, true, ""); 
 @>     if(assets > maxAssets) revert slippageError();
        return assets;
    }

 @>   function withdraw(uint256 assets, address receiver, address owner, uint256 maxShares) external override returns (uint256) { 
        (, uint256 shares) = _withdraw(receiver, owner, assets, false);
 @>     require(shares < maxShares, "slippage err");
        return shares;
    }

 @>    function redeem(uint256 shares, address receiver, address owner, uint256 minAssets) external override returns (uint256) { //@note should include slippage
        (uint256 assets,) = _withdraw(receiver, owner, shares, true);
@>       require(minAssets < assets, "slippage error");
        return assets;
    }

Assessed type

Invalid Validation

c4-pre-sort commented 4 months ago

0xEVom marked the issue as sufficient quality report

c4-pre-sort commented 4 months ago

0xEVom marked the issue as duplicate of #281

c4-judge commented 3 months ago

jhsagd76 changed the severity to QA (Quality Assurance)

c4-judge commented 3 months ago

This previously downgraded issue has been upgraded by jhsagd76

c4-judge commented 3 months ago

jhsagd76 changed the severity to QA (Quality Assurance)