Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Innocent Blonde Finch - Potencial Denial of service in LidoVault::deposit #153

Open
sherlock-admin2 opened this issue Sep 21, 2024 · 0 comments

Comments

@sherlock-admin2
Copy link
Contributor

Innocent Blonde Finch

Medium

Potencial Denial of service in LidoVault::deposit

Summary

The deposit function in the LidoVault contract exhibits a Denial of Service (DOS) vulnerability due to its implementation of minimum deposit requirements. This vulnerability manifests in two critical ways: firstly, it potentially excludes smaller depositors from participating in the vault, particularly on the fixed side where the minimum deposit is calculated as a percentage of the total capacity. Secondly, it can prevent the vault from reaching full capacity by disallowing deposits when the remaining capacity falls below the minimum fixed deposit amount but is greater than zero. These issues stem from well-intentioned checks designed to ensure meaningful participation, but their current implementation could inadvertently limit the vault's accessibility and efficiency. This vulnerability could significantly impact the vault's liquidity and overall performance to a diverse user base, potentially undermining the contract's core functionality.

Vulnerability Detail

The deposit function implements two levels of minimum deposit checks:

https://github.com/sherlock-audit/2024-08-saffron-finance/blob/main/lido-fiv/contracts/LidoVault.sol#L333

  1. A global minimum deposit amount for all deposits:
    require(msg.value >= minimumDepositAmount, "MDA");

https://github.com/sherlock-audit/2024-08-saffron-finance/blob/main/lido-fiv/contracts/LidoVault.sol#L339

  1. A specific minimum for fixed-side deposits, calculated as a percentage of the total fixed side capacity:
    uint256 minimumFixedDeposit = fixedSideCapacity.mulDiv(minimumFixedDepositBps, 10_000);
    require(amount >= minimumFixedDeposit, "MFD");

Additionally, there's a check to ensure the remaining capacity after a deposit is either zero or above the minimum fixed deposit:

These checks, while intended to ensure meaningful participation, can lead to exclusion of smaller depositors and prevent the vault from reaching full capacity.

Impact

The impact of this vulnerability is twofold:

  1. Smaller depositors may be unable to participate in the vault, particularly on the fixed side where the minimum deposit is a percentage of the total capacity. This could significantly limit the accessibility of the vault to a broader user base.

  2. The vault may be unable to reach full capacity. If the remaining capacity falls below the minimum fixed deposit amount but is greater than zero, no further deposits can be made, leaving the vault partially unfilled.

These issues could lead to reduced liquidity in the vault, potentially affecting its overall performance and attractiveness to users.

Code Snippet

function deposit(uint256 side) external payable {
require(msg.value >= minimumDepositAmount, "MDA");
uint256 amount = msg.value;
if (side == FIXED) {
uint256 minimumFixedDeposit = fixedSideCapacity.mulDiv(minimumFixedDepositBps, 10_000);
require(amount >= minimumFixedDeposit, "MFD");
uint256 remainingCapacity = fixedSideCapacity - fixedETHDepositTokenTotalSupply - amount;
require(remainingCapacity == 0 || remainingCapacity >= minimumFixedDeposit, "RC");
// ... rest of the function
}
// ... rest of the function
}

Tool used

Manual Review

Recommendation

To mitigate this vulnerability, consider implementing the following changes:

  1. Carefully review and set the minimumDepositAmount and minimumFixedDepositBps values to ensure they align with the target user base and don't unnecessarily exclude potential participants.

  2. Consider removing or adjusting the final capacity check to allow for full capacity to be reached:

   // Remove this check or adjust it to allow smaller final deposits
-     require(remainingCapacity == 0 || remainingCapacity >= minimumFixedDeposit, "RC");

By implementing these recommendations, the contract can maintain meaningful participation while increasing accessibility and ensuring the vault can reach full capacity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant