GiantMevAndFeesPool and StakingFundsVault: Miscalculation of rewards when transferring tokens leads to loss of rewards

[code423n4]

Lines of code

https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/GiantMevAndFeesPool.sol#L146
https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/GiantMevAndFeesPool.sol#L170
https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/StakingFundsVault.sol#L343

Vulnerability details

Impact

This issue exists in GiantMevAndFeesPool as well as in StakingFundsVault.

From here on I will explain the issue based on the GiantMevAndFeesPool but the explanation can be transferred easily to the StakingFundsVault.

The holders of a GiantMevAndFeesPools GiantLP tokens receive a share of the staking rewards.

In order to handle the rewards correctly in case the tokens are transferred, the GiantMevAndFeesPool implements the beforeTokenTransfer (https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/GiantMevAndFeesPool.sol#L146) and afterTokenTransfer (https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/GiantMevAndFeesPool.sol#L170) functions.

The beforeTokenTransfer function sends all pending rewards to the sender and receiver of the token transfer.

The afterTokenTransfer functions updates the amount of claimed rewards for the receiver of the token transfer. This is done to disallow the receiver to claim rewards for tokens that the sender has already claimed rewards for.

The problem is that the claimed rewards for the sender are not adapted in the afterTokenTransfer.

The claimed rewards for the sender should be lowered to reflect the sender's now lower balance.

This causes the sender to miss out on future rewards (-> loss of funds).

Proof of Concept

Consider the following scenario:

• Alice owns 5 GiantLP tokens
• Bob owns 5 GiantLP tokens
• No rewards were claimed so far and 1 ETH per GiantLP tokens is the current reward

1. Alice wants to send 1 GiantLP to Bob
2. The beforeTokenTransfer is called and rewards for Alice and Bob are claimed (5 ETH for Alice and 5 ETH for Bob)
3. The transfer is executed. Alice owns 4 GiantLP. Bob owns 6 GiantLP
4. The afterTokenTransfer function is executed. Bob's claimed amount will be set to 6 ETH, reflecting his new balance of 6 GiantLP.
5. Alice's claimed amount however will remain at 5 ETH. This means that she will miss out on future rewards. She will only start earning rewards when the reward per GiantLP reaches 5 ETH / 4 GiantLP = 1.25 ETH / GiantLP.

Tools Used

VSCode

Recommended Mitigation Steps

In the afterTokenTransfer function call _setClaimedToMax(_from):

-    function afterTokenTransfer(address, address _to, uint256) external {
+    function afterTokenTransfer(address _from, address _to, uint256) external {
         require(msg.sender == address(lpTokenETH), "Caller is not giant LP");
         _setClaimedToMax(_to);
+        _setClaimedToMax(_from);
     }

[c4-judge]

dmvt marked the issue as duplicate of #179

[c4-judge]

dmvt marked the issue as duplicate of #178

[c4-judge]

dmvt marked the issue as satisfactory