Fjord Token Staking
Fjord
DeFiFoundry
20,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Gas Limit and DOS Vulnerability in unstakeAll() Function

mrudenko

Summary

The unstakeAll() function in the FjordStaking contract contains a loop that iterates over all active deposits of a user. If a user has a large number of active deposits, this loop could exceed the block gas limit, causing the transaction to fail. This situation could be exploited by malicious users to create a denial-of-service (DOS) scenario, preventing other users from being able to successfully unstake their tokens.

Vulnerability Details

uint256[] memory activeDeposits = getActiveDeposits(msg.sender);
if (activeDeposits.length == 0) revert NoActiveDeposit();
​
for (uint16 i = 0; i < activeDeposits.length; i++) {
uint16 epoch = uint16(activeDeposits[i]);
DepositReceipt storage dr = deposits[msg.sender][epoch];
​
if (dr.epoch == 0 || currentEpoch - epoch <= lockCycle) continue;
​
totalStakedAmount += dr.staked;
​
// no vested staked and stake is 0 then delete the deposit
if (dr.vestedStaked == 0) {
delete deposits[msg.sender][epoch];
_activeDeposits[msg.sender].remove(epoch);
} else {
// still have vested staked, then only delete the staked
dr.staked = 0;
}
}
​
totalStaked -= totalStakedAmount;
userData[msg.sender].totalStaked -= totalStakedAmount;
​
fjordToken.transfer(msg.sender, totalStakedAmount);
points.onUnstaked(msg.sender, totalStakedAmount);
​
// emit event
emit UnstakedAll(
msg.sender, totalStakedAmount, activeDeposits, getActiveDeposits(msg.sender)
);
​

https://github.com/Cyfrin/2024-08-fjord/blob/0312fa9dca29fa7ed9fc432fdcd05545b736575d/src/FjordStaking.sol#L570-L607

In this function, the contract loops through all active deposit epochs of a user to unstake them. If a user has a very high number of active deposits, this loop could consume more gas than is allowed per block. As a result, the transaction would fail, leading to a situation where it is impossible to unstake tokens.

Impact

The main impact of this vulnerability is the potential for denial-of-service (DOS) attacks:

  • User Lockout: A user with many active deposits might be unable to unstake their tokens, effectively locking their funds.

  • Systemic Risk: An attacker could exploit this by generating many small deposits, creating a large number of active deposits. This would make the unstakeAll() function fail due to gas limits, thus preventing other users from using the function successfully.
    This issue not only affects individual users but could also degrade the overall usability and reliability of the staking contract, leading to a loss of confidence in the system.

Tools Used

Recommendations

Allow Partial Unstaking: Implement a function that allows users to unstake a specific subset of their deposits, rather than all at once. This would give users the flexibility to manage their gas usage and prevent complete lockout scenarios.

Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.