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

13 stars 10 forks source link

Lack of Slippage Protection in `withdraw`/`redeem` Function of the Vault #452

Open c4-bot-1 opened 8 months ago

c4-bot-1 commented 8 months ago

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L372-L375 https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L378-L381 https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/V3Vault.sol#L1175-L1179

Vulnerability details

Impact

The lendExchangeRateX96 is dependent upon the utilization rate of debt in the Vault.

(, uint256 available,) = _getAvailableBalance(oldDebtExchangeRateX96, oldLendExchangeRateX96);

uint256 debt = _convertToAssets(debtSharesTotal, oldDebtExchangeRateX96, Math.Rounding.Up);

(uint256 borrowRateX96, uint256 supplyRateX96) = interestRateModel.getRatesPerSecondX96(available, debt);

Due to unfavorable timing, this rate can change after a user has called redeem or withdraw but prior to the transaction being confirmed. Since the Vault is lacking any slippage checks, the user may receive fewer assets or burn more shares than expected.

Proof of Concept

Withdraw and redeem functions contain no slippage checks.

function withdraw(uint256 assets, address receiver, address owner) external override returns (uint256) {
    (, uint256 shares) = _withdraw(receiver, owner, assets, false);
    return shares;
}

function redeem(uint256 shares, address receiver, address owner) external override returns (uint256) {
    (uint256 assets,) = _withdraw(receiver, owner, shares, true);
    return assets;
}

Tools Used

Manual Review

Recommended Mitigation Steps

Both withdraw/redeem functions should include slippage protection parameters provided by the users (either minimum amount out for redeem function or maximum shares in for withdraw function).

Assessed type

Context

c4-pre-sort commented 7 months ago

0xEVom marked the issue as sufficient quality report

c4-pre-sort commented 7 months ago

0xEVom marked the issue as duplicate of #281

c4-judge commented 7 months ago

jhsagd76 changed the severity to QA (Quality Assurance)

c4-judge commented 7 months ago

jhsagd76 marked the issue as grade-b

c4-judge commented 7 months ago

This previously downgraded issue has been upgraded by jhsagd76

c4-judge commented 7 months ago

jhsagd76 changed the severity to QA (Quality Assurance)