20,000 USDC
View results
Submission Details
Severity: low
Valid

Staking tokens transfer to the staking contract will make a fraction of WETH rewards be lost forever

Summary

If an amount of tokens are transferred directly into the stacking contract, instead of using the deposit() function, those tokens will be used for the calculation of rewards.

Vulnerability Details

On Staking.sol for the calculation of rewards the balance of staking tokens of the contract is used. That means that not only tokens deposited with the function deposit() are considered. Transferred tokens, using ERC20 transfer() function are considered as well.

It is possible to grief the rewards for the stakers being it in purpose or by mistake. It would be costly to the attacker, but since the issue is a easy fix, it is recommended that the amount of tokens stacked is obtained by a storage variable.

Impact

A fraction of rewards will be lost on the staking contract, impossible to retrieve.

Tools Used

Manual Review

Recommendations

Instead of using the ERC20 balance, use a storage variable to keep track of staked tokens.

diff --git a/src/Staking.sol b/src/Staking.sol

@@ -20,6 +20,9 @@ contract Staking is Ownable {

    /// @notice mapping of user balances
    mapping(address => uint256) public balances;
+    
+    uint256 public totalStakedBalances;
+
    /// @notice mapping of user claimable rewards
    mapping(address => uint256) public claimable;
    
@@ -28,6 +28,7 @@ contract Staking is Ownable {
    /// @notice the reward token
    IERC20 public immutable WETH;

    constructor(address _token, address _weth) Ownable(msg.sender) {
        TKN = IERC20(_token);
        WETH = IERC20(_weth);
@@ -38,6 +43,7 @@ contract Staking is Ownable {
    function deposit(uint _amount) external {
        TKN.transferFrom(msg.sender, address(this), _amount);
        updateFor(msg.sender);
+        totalStakedBalances += _amount;
        balances[msg.sender] += _amount;
    }

@@ -46,6 +52,7 @@ contract Staking is Ownable {
    function withdraw(uint _amount) external {
        updateFor(msg.sender);
        balances[msg.sender] -= _amount;
+        totalStakedBalances -= _amount;
        TKN.transfer(msg.sender, _amount);
    }
    
@@ -56,10 +56,20 @@ contract Staking is Ownable {
        claimable[msg.sender] = 0;
        balance = WETH.balanceOf(address(this));
    }

    function update() public {
-        uint256 totalSupply = TKN.balanceOf(address(this));
+        uint256 totalSupply = totalStakedBalances;
        if (totalSupply > 0) {
            uint256 _balance = WETH.balanceOf(address(this));
            if (_balance > balance) {

Support

FAQs

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