SyncCode2017 - Users funds will be stuck if many users request for withdrawal from the fixed side when the vault has started due to two unbounded array lengths in `LidoVault::finalizeVaultOngoingFixedWithdrawals`. #147
Users funds will be stuck if many users request for withdrawal from the fixed side when the vault has started due to two unbounded array lengths in LidoVault::finalizeVaultOngoingFixedWithdrawals.
Summary
When a user calls LidoVault::finalizeVaultOngoingFixedWithdrawals to request for withdrawal from the fixed side when the vault has started, each of the 2 arrays in line 599 and line 1174 will be looped through. With sufficient number of withdrawals pending, the gas needed to complete this transaction will be greater than the block gas limit (30 million gas) and every transaction calling this function will fail.
Two unbounded arrays will be looped through when LidoVault::finalizeVaultOngoingFixedWithdrawals is called by a user. The unbounded arrays are in line 599 and line 1174 as shown in the summary section.
Internal pre-conditions
No response
External pre-conditions
No response
Attack Path
No response
Impact
Funds of users requesting for withdrawals from an ongoing vault will be stuck due to unusually high gas (higher than block gas limit) required to complete the transaction. Especially when this contract is running on Ethereum mainnet where gas fees are very high.
When a user calls LidoVault::finalizeVaultOngoingFixedWithdrawals to request for withdrawal, each of the 2 arrays will be looped through. With sufficient number of withdrawals pending, the gas needed to complete this transaction will be greater than the block gas limit (30 million gas) and every transaction calling this function will fail.
Mitigation
The contract follows the steps below for a withdrawal from ongoing fixed-side vault to be finalised;
user calls LidoVault::withdraw to place request for withdrawal. The user request is not sent to Lido immediately, it is queued up along with other requests;
all the queued up requests are sent when a user calls LidoVault::finalizeVaultOngoingFixedWithdrawals. Each request is sent individually in a loop. So a long loop of requests is expected to be processed in this one transaction.
This logic is erroneous. Besides funds being stuck due to unusually high gas, a user is made to pay gas fees for other users requests to be sent to Lido. This logic must reviewed and revised.
SyncCode2017
High
Users funds will be stuck if many users request for withdrawal from the fixed side when the vault has started due to two unbounded array lengths in
LidoVault::finalizeVaultOngoingFixedWithdrawals.Summary
When a user calls
LidoVault::finalizeVaultOngoingFixedWithdrawalsto request for withdrawal from the fixed side when the vault has started, each of the 2 arrays in line 599 and line 1174 will be looped through. With sufficient number of withdrawals pending, the gas needed to complete this transaction will be greater than the block gas limit (30 million gas) and every transaction calling this function will fail.https://github.com/sherlock-audit/2024-08-saffron-finance/blob/main/lido-fiv/contracts/LidoVault.sol#L599
https://github.com/sherlock-audit/2024-08-saffron-finance/blob/main/lido-fiv/contracts/LidoVault.sol#L1174
Funds of these users will be stuck.
Root Cause
Two unbounded arrays will be looped through when
LidoVault::finalizeVaultOngoingFixedWithdrawalsis called by a user. The unbounded arrays are in line 599 and line 1174 as shown in the summary section.Internal pre-conditions
No response
External pre-conditions
No response
Attack Path
No response
Impact
Funds of users requesting for withdrawals from an ongoing vault will be stuck due to unusually high gas (higher than block gas limit) required to complete the transaction. Especially when this contract is running on Ethereum mainnet where gas fees are very high.
PoC
30 million gas is the maximum gas limit per block in the Ethereum blockchain according to ethereum.org. Please click the link below. https://ethereum.org/en/developers/docs/gas/#block-size:~:text=fee%20and%20tip.-,Block%20size,-Each%20block%20has
As the number of users and withdrawal requests on the fixed side grows so also the array lengths in line 599 and line 1174 grow and gas needed to complete the withdrawal transaction. https://github.com/sherlock-audit/2024-08-saffron-finance/blob/main/lido-fiv/contracts/LidoVault.sol#L599
https://github.com/sherlock-audit/2024-08-saffron-finance/blob/main/lido-fiv/contracts/LidoVault.sol#L1174
When a user calls
LidoVault::finalizeVaultOngoingFixedWithdrawalsto request for withdrawal, each of the 2 arrays will be looped through. With sufficient number of withdrawals pending, the gas needed to complete this transaction will be greater than the block gas limit (30 million gas) and every transaction calling this function will fail.Mitigation
The contract follows the steps below for a withdrawal from ongoing fixed-side vault to be finalised;
LidoVault::withdrawto place request for withdrawal. The user request is not sent to Lido immediately, it is queued up along with other requests;LidoVault::finalizeVaultOngoingFixedWithdrawals. Each request is sent individually in a loop. So a long loop of requests is expected to be processed in this one transaction.This logic is erroneous. Besides funds being stuck due to unusually high gas, a user is made to pay gas fees for other users requests to be sent to Lido. This logic must reviewed and revised.