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

`Stake` function may be reentered causing loss of Funds

silentwalker

Summary

The stake function does not follow the check-effects-interaction pattern. It adds the users' staked amount before transferring the tokens from the user, making it possible for a malicious user to call the stake function with a malicious contract that has a fallback function to reenter the function. This can increase their stake amount without transferring the tokens.

Vulnerability Details

Steps to reproduce the hack

  • A malicious user will call the stake function with a malicious contract several times to increase their deposit amount in the deposit receipt, thereby increasing their amount of FJORD tokens.

Stake Function

DepositReceipt storage dr = deposits[msg.sender][currentEpoch];
​
if (dr.epoch == 0) {
dr.staked = _amount;
dr.epoch = currentEpoch;
_activeDeposits[msg.sender].add(currentEpoch);
} else {
dr.staked += _amount;
}
​
newStaked += _amount;

Since the points will not be given to the user, only the amount will be added to their deposits.

  • The user will call the unstake function and withdraw all the FJORD tokens from the contract.

Impact

Tokens staked by other users and protocol tokens will be stolen.

Tools Used

Manual Review

Recommendations

Use ReentrancyGuard from OpenZeppelin, which allows you to add a modifier, e.g., nonReentrant, to functions that may otherwise be vulnerable. Follow the check-effects-interactions pattern as shown below.

function stake(uint256 _amount) external checkEpochRollover redeemPendingRewards {
// CHECK
if (_amount == 0) revert InvalidAmount();
​
// EFFECT
userData[msg.sender].unredeemedEpoch = currentEpoch;
​
DepositReceipt storage dr = deposits[msg.sender][currentEpoch];
+ fjordToken.safeTransferFrom(msg.sender, address(this), _amount);
​
if (dr.epoch == 0) {
dr.staked = _amount;
dr.epoch = currentEpoch;
_activeDeposits[msg.sender].add(currentEpoch);
} else {
dr.staked += _amount;
}
​
newStaked += _amount;
​
// INTERACT
- fjordToken.safeTransferFrom(msg.sender, address(this), _amount);
points.onStaked(msg.sender, _amount);
​
emit Staked(msg.sender, currentEpoch, _amount);
}
Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Lack of quality

Support

FAQs

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