DeFiFoundry
20,000 USDC
View results
Submission Details
Severity: high
Invalid

Reentrancy Attack Vector in Unstaking and Claiming Operations

Description:
Several functions in the contract allow external calls to transfer tokens and update user data, making the contract potentially vulnerable to reentrancy attacks. Specifically, functions like unstake, unstakeVested, unstakeAll, claimReward, and completeClaimRequest allow users to withdraw or claim tokens without reentrancy protection.

Location:

src/FjordStaking.sol

  • unstake function (Line 449)

  • unstakeVested function (Line 512)

  • unstakeAll function (Line 570)

  • claimReward function (Line 616)

  • completeClaimRequest function (Line 662)

Issue:
These functions interact with external token contracts (using safeTransfer) before all state changes are finalized. If a reentrant call is made by the token contract or any contract called by the token transfer, it could lead to inconsistent or exploited state changes, such as double withdrawals.

Impact:

  • A reentrancy attack could allow a malicious user to repeatedly withdraw funds or claim rewards, potentially draining the contract's balance or receiving more tokens than intended.

Tools used: Manual Review.

Recommendations:
Implement the nonReentrant modifier from OpenZeppelin's ReentrancyGuard or a custom reentrancy guard to protect these functions. Ensure that all state changes are finalized before any external calls.

Potential changes:

  • Add the nonReentrant modifier to the unstake, unstakeVested, unstakeAll, claimReward, and completeClaimRequest functions.

Changes needed for which line of code:

  • Add nonReentrant Modifier

import "@openzeppelin/contracts/security/ReentrancyGuard.sol";
contract FjordStaking is ISablierV2LockupRecipient, ReentrancyGuard {
// ... existing code ...
// Add the modifier to the functions
function unstake(uint16 _epoch, uint256 _amount)
external
checkEpochRollover
redeemPendingRewards
nonReentrant
returns (uint256 total)
{ ... }
function unstakeVested(uint256 _streamID)
external
checkEpochRollover
redeemPendingRewards
nonReentrant
{ ... }
function unstakeAll()
external
checkEpochRollover
redeemPendingRewards
nonReentrant
returns (uint256 totalStakedAmount)
{ ... }
function claimReward(bool _isClaimEarly)
external
checkEpochRollover
redeemPendingRewards
nonReentrant
returns (uint256 rewardAmount, uint256 penaltyAmount)
{ ... }
function completeClaimRequest()
external
checkEpochRollover
redeemPendingRewards
nonReentrant
returns (uint256 rewardAmount)
{ ... }
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Lack of quality

Support

FAQs

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