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

Only the `rewardAdmin` is expected to call `FjordStaking.addReward` function but anyone can add rewards (FjordTokens) to the `FjordStaking` contract

Summary

Only the rewardAdmin is expected to call FjordStaking.addReward function but anyone can add rewards (FjordTokens) to the FjordStaking contract

Vulnerability Details

The FjordStaking.addReward function is access controlled since only the rewardAdmin can call this function. But this access control can be bypassed since anyone can deposit rewards to this contract and distribute them among the stakers at thier discretion.

This issue is prevalent due to the way the pendingRewards are calculated in the FjordStaking._checkEpochRollover function. It uses the fjordToken.balanceOf(address(this)) to calculate the balance of the contract irrespective of the way the fjordTokens have been deposited to the FjordStaking contract. And this value is used to calculate the pendingRewards which is used to calculate the totalRewards state variable.

Impact

Hence any external party has the ability to deposit fjordTokens in to the FjordStaking contract and manipulate the pendingRewards which will update the rewardPerToken[epoch] mapping values and the totalRewards state variable. As a result this will impact the reward distribution of all the stakers of the Fjord protocol. This is against the expected behavior of the FjordProtocol since only the rewardAdmin is expected to add rewards (fjordTokens) to the FjordStaking contract.

https://github.com/Cyfrin/2024-08-fjord/blob/main/src/FjordStaking.sol#L699-L703
https://github.com/Cyfrin/2024-08-fjord/blob/main/src/FjordStaking.sol#L755-L768

Tools Used

Manual Review and VSCode

Recommendations

Hence if only the rewardAdmin is allowed to transfer the fjordTokens to the FjordStaking contract it is recommended to keep internal accounting of the amount of rewards transferred by the rewardAdmin via the addReward() function, and use that amount inplace of fjordToken.balanceOf(address(this)) during the pendingRewards calculation. This internal accounting amount for fjordTokens should be updated accordingly during the respective transactions in the FjordStaking contract. This will ensure only the rewardAdmin can transfer the fjordTokens to the FjordStaking contract and no one else can bypass the access control.

Updates

Lead Judging Commences

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.