LSD Network - Stakehouse contest - HE1M's results

Lines of code

https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/syndicate/Syndicate.sol#L228

Vulnerability details

Impact

During staking in Syndicate to get a portion of syndicate rewards, the mapping sETHUserClaimForKnot is not properly set. So, a malicious user can drain ETH in the syndicate completely.

Proof of Concept

Suppose Bob (malicious user) stakes 1 sETH by calling stake(...) with the following parameters:

• _blsPubKeys: [a valid BLS public key]
• _sETHAmounts: [10^18] which is equal to 1 ether
• _onBehalfOf: Bob's address https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/syndicate/Syndicate.sol#L203

So, we will have the following important mappings set as:

• sETHStakedBalanceForKnot[BLS][Bob] = 10^18
• sETHUserClaimForKnot[BLS][Bob] = (10^18 * accumulatedETHPerFreeFloatingShare) / PRECISION

Now, if Bob calls claimAsStaker(...), he will not receive any reward. https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/syndicate/Syndicate.sol#L285

Because in the function calculateUnclaimedFreeFloatingETHShare, the values of important variables will be:

• stakedBal = 10^18
• userShare = (accumulatedETHPerShare * 10^18) / PRECISION
• userShare - sETHUserClaimForKnot[_blsPubKey][_user] = 0 So, unclaimedUserShare will be equal to zero.

Bob calls the function stake(...) again with the following parameters:

• _blsPubKeys: [the same BLS public key]
• _sETHAmounts: [2 * 10^9] which is equal to 2 gwei
• _onBehalfOf: Bob's address

So, we will have the following important mappings set as:

• sETHStakedBalanceForKnot[BLS][Bob] = 10^18 + 2 * 10^9
• sETHUserClaimForKnot[BLS][Bob] = (2 * 10^9 * newAccumulatedETHPerFreeFloatingShare) / PRECISION

Now, if Bob calls claimAsStaker(...), he will receive award, because we have:

• stakedBal = 10^18 + 2 * 10^9
• userShare = (newAccumulatedETHPerShare * (10^18 + 2 * 10^9)) / PRECISION
• userShare - sETHUserClaimForKnot[_blsPubKey][_user] > 0 So, unclaimedUserShare will not be equal to zero. Then, this amount will be transferred to Bob: https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/syndicate/Syndicate.sol#L668

Please note that the accumulatedETHPerShare will be increased by each deposit (as expected), but the vulnerability is that sETHUserClaimForKnot[BLS][Bob] is not accumulated when Bob stakes to protect against claiming historical reward. Instead, it is set to the exact amount of Bob's stake at that moment.

In summary, Bob stakes 1 sETH, and as expected he should not be able to get reward instantly (he should wait until the ETH per free floating shares accumulates, then he qualifies to claim reward). So, then Bob stakes much smaller value, just to set the value of claim mapping to a wrong small value. Then, Bob can get reward on his first stake of 1 sETH. In other words, by the second small value of stake, Bob qualifies himself as getting the reward on his firs stake instantly, although no ETH per free floating shares is accumulated yet. This attack means claiming on historical reward.

In the following function, Bob first stakes 1 sETH, then in the loop stakes 2 gwei and claim the reward again and again until draining the syndicate:

function attack(bytes calldata _blsPubKeys) public {
    bytes[] bls = new bytes[](1);
    bls[0] = _blsPubKeys;
    uint256[] sETH = new uint256[](1);
    sETH[0] = 1 ether;

    syndicate.stake(bls, sETH, address(this));

    sETH[0] = 2 * 10**9;

    while (address(syndicate).balance > 0) {
        syndicate.stake(bls, sETH, address(this));
        syndicate.claimAsStaker(address(this), bls);
    }
}

Tools Used

Recommended Mitigation Steps

The solution is to modify the line 228: https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/syndicate/Syndicate.sol#L228

sETHUserClaimForKnot[_blsPubKey][_onBehalfOf] += (_sETHAmount * accumulatedETHPerFreeFloatingShare) / PRECISION;

Lines of code

https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/syndicate/Syndicate.sol#L245

Vulnerability details

Impact

If a user stakes some sETH, and after some time decides to unstake some amount of sETH, later s/he will not be qualified or be less qualified to claim ETH on the remaining staked sETH.

Proof of Concept

Suppose Alice stakes 5 sETH by calling stake(...). https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/syndicate/Syndicate.sol#L203 So, we will have:

• sETHUserClaimForKnot[BLS][Alice] = (5 * 10^18 * accumulatedETHPerFreeFloatingShare) / PRECISION
• sETHStakedBalanceForKnot[BLS][Alice] = 5 * 10^18
• sETHTotalStakeForKnot[BLS] += 5 * 10^18

Later, Alice decides to unstake 3 sETH by calling unstake(...). https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/syndicate/Syndicate.sol#L245

So, all ETH owed to Alice will be paid: https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/syndicate/Syndicate.sol#L257

Then, we will have:

• sETHUserClaimForKnot[BLS][Alice] = (5 * 10^18 * accumulatedETHPerFreeFloatingShare) / PRECISION
• sETHStakedBalanceForKnot[BLS][Alice] = 2 * 10^18
• sETHTotalStakeForKnot[BLS] -= 3 * 10^18

It is clear that the mapping sETHStakedBalanceForKnot is decreased as expected, but the mapping sETHUserClaimForKnot is not changed. In other words, the mapping sETHUserClaimForKnot is still holding the claimed amount based on the time 5 sETH were staked.

If, after some time, the ETH is accumulated per free floating share for the BLS public key that Alice was staking for, Alice will be qualified to some more ETH to claim (because she has still 2 sETH staked).

If Alice unstakes by calling unstake(...) or claim ETH by calling claimAsStaker(...), in both calls, the function calculateUnclaimedFreeFloatingETHShare will be called to calculate the amount of unclaimed ETH: https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/syndicate/Syndicate.sol#L652

In this function, we will have:

• stakedBal = sETHStakedBalanceForKnot[BLS][Alice] = 2 * 10^18
• userShare = (newAccumulatedETHPerShare * stakedBal) / PRECISION

The return value which is unclaimed ETH will be:

userShare - sETHUserClaimForKnot[BLS][Alice] = 
(newAccumulatedETHPerShare * 2 * 10^18) / PRECISION - (5 * 10^18 * accumulatedETHPerFreeFloatingShare) / PRECISION

This return value is not correct (it is highly possible to be smaller than 0, and as a result Alice can not claim anything), because the claimed ETH is still based on the time when 5 sETH were staked, not on the time when 2 sETH were remaining/staked.

The vulnerability is that during unstaking, the mapping sETHUserClaimForKnot is not updated to the correct value. In other words, this mapping is updated in _claimAsStaker, but it is updated based on 5 sETH staked, later when 3 sETH are unstaked, this mapping should be again updated based on the remaing sETH (which is 2 sETH).

As a result, Alice can not claim ETH or she will qualify for less amount.

Tools Used

Recommended Mitigation Steps

The following line should be added on line 274:

sETHUserClaimForKnot[_blsPubKey][msg.sender] =
                (accumulatedETHPerShare * sETHStakedBalanceForKnot[_blsPubKey][msg.sender]) / PRECISION

https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/syndicate/Syndicate.sol#L274

Lines of code

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

Vulnerability details

Impact

When minting LPToken the rewards distribution to From is excluded because it is address(0). But during burning LPToken, the rewards distribution to To is not excluded. This results in revert of the transaction whenever burning the LPToken.

Proof of Concept

In the GiantMevAndFeesPool and StakingFundsVault, when LPToken is transferred, the hooks beforeTokenTransfer and afterTokenTransfer will be executed:

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

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

During minting the LPToken, From is address(0). So, in beforeTokenTransfer, it is excluding distributing rewards to From. This is the correct procedure.

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

During burning the LPToken, To is address(0). But, in beforeTokenTransfer, it is not excluding distributing rewards to To. This is not correct.

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

This will result in revert of the transaction (burning LPToken), because it is not allowed to have address(0) recepient. https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/SyndicateRewardsProcessor.sol#L57

Tools Used

Recommended Mitigation Steps

Better to have the following function modified as:

function beforeTokenTransfer(
    address _from,
    address _to,
    uint256
) external {
    require(msg.sender == address(lpTokenETH), "Caller is not giant LP");
    updateAccumulatedETHPerLP();

    // Make sure that `_from` gets total accrued before transfer as post transferred anything owed will be wiped
    if (_from != address(0)) {
        _distributeETHRewardsToUserForToken(
            _from,
            address(lpTokenETH),
            lpTokenETH.balanceOf(_from),
            _from
        );
    }

    // Make sure that `_to` gets total accrued before transfer as post transferred anything owed will be wiped
    if (_to != address(0)) {
        _distributeETHRewardsToUserForToken(
            _to,
            address(lpTokenETH),
            lpTokenETH.balanceOf(_to),
            _to
        );
    }
}

Lines of code

https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/SyndicateRewardsProcessor.sol#L63

Vulnerability details

Impact

During distributing the reward, the claimed mapping (that tracks total ETH claimed by a given address for a given token) is not correctly set to the max claimed value or increased, instead it is set to the amount that is distributed at that moment. This is a critical vulnerability as a malicious user can claim for the reward again and again without being prevented and drain the protocol.

Proof of Concept

Suppose Bob (malicious user) deposits some ETH by calling depositETHForStaking. So, accumulatedETHPerLPShare will be updated, and equivalent LPToken will be minted for the user. Since Bob before did not have any balance of LPToken, no reward will be distributed to him. Moreover, the mapping claimed will be set to the max value, which means Bob is not qualified now to get any rewards. claimed[msg.sender][address(tokenForKnot)] = (tokenForKnot.balanceOf(msg.sender) * accumulatedETHPerLPShare) / PRECISION;

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

Later, suppose the accumulatedETHPerLPShare is increased to new value accumulatedETHPerLPShareNew.

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;
            }
        }
    }

So, the max claimed value for Bob is now equal to (tokenForKnot.balanceOf(msg.sender) * accumulatedETHPerLPShareNew) / PRECISION. For simplicity assume:

• (tokenForKnot.balanceOf(msg.sender) * accumulatedETHPerLPShare) / PRECISION = 5 ETH
• (tokenForKnot.balanceOf(msg.sender) * accumulatedETHPerLPShareNew) / PRECISION = 6 ETH

Now Bob calls claimRewards in StakingFundsVault: https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/StakingFundsVault.sol#L199

Then, during reward distribution to Bob, the value of due will be 6 ETH - 5 ETH equals to 1 ETH. Then this 1 ETH will be transferred to Bob, and the mapping claimed[Bob][_token] = 1 ETH.

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);
            }
        }
    }

https://github.com/code-423n4/2022-11-stakehouse/blob/39a3a84615725b7b2ce296861352117793e4c853/contracts/liquid-staking/SyndicateRewardsProcessor.sol#L51

Then again Bob calls claimRewards, so again he will received due amount which is equal to 6 ETH - 1 ETH equals to 5 ETH, and the mapping claimed[Bob][_token] = 5 ETH. So, by calling this function again and again he can drain all the funds.

The vulnerability is that the mapping claimed[_user][_token] is not set to the max claimed value or is not increased according to the payment after distributing the rewards.

Tools Used

Recommended Mitigation Steps

The function should be modified as:

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 maxClaimedValue = ((accumulatedETHPerLPShare * balance) /
            PRECISION);
        uint256 due = maxClaimedValue - claimed[_user][_token];
        if (due > 0) {
            claimed[_user][_token] = maxClaimedValue;

            totalClaimed += due;

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

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

OR

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);
        }
    }
}

Lines of code

https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/GiantPoolBase.sol#L96

Vulnerability details

Impact

It is possible to apply grieving attack to prevent any user from interacting with lpTokenETH by extending the delay 1 day again and again. Interacting with lpTokenETH means any function call on the token that triggers the internal function _afterTokenTransfer.

Proof of Concept

A user can withdraw vault LP token by burning his Giant LPT if the last time interacted with Giant LPT is more than 1 day ago.

https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/GiantPoolBase.sol#L80 https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/GiantPoolBase.sol#L96

An attacker can deposit ETH by calling depositETH, and as a result lpTokenETH (which is Giant LPT) will be minted for him. https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/GiantPoolBase.sol#L42

Then the attacker should wait for 1 day to be able to transfer lpTokenETH to other addresses.

Now, if any user calls withdrawLPTokens, the attacker front-run on the user and transfers the smallest value of lpTokenETH to the user just to increase the lastInteractedTimestamp of the user. https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/GiantLP.sol#L43

By doing so, the user can not withdraw vault LP token, and should wait for one more day. The attacker can extend this attack more by transferring some values of lpTokenETH to his other addresses (addressA, addressB, addressC, and so on). So, later, the attacker by using these addresses (addressA, addressB, addressC, and so on) can extend the delay of any user who intends to interact with lpTokenETH.

The same scenario can be applied to the _lpToken in SavETHVault, and preventing a user from burning LP token in exchange of ETH or dETH by extending the delay again and again: https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/SavETHVault.sol#L141

Tools Used

Recommended Mitigation Steps

Maybe one solution is that if interaction with lpTokenETH is more than a threshold, then it should increase the lastInteractedTimestamp. In this case, the attacker must transfer higher values to the user to apply this grieving attack, that is not financially reasonable for the attacker.

function _afterTokenTransfer(
    address _from,
    address _to,
    uint256 _amount
) internal override {
    if (amount > 0.1 ether) { // more than this threshold, the last intracted time stamp should be updated
        lastInteractedTimestamp[_from] = block.timestamp;
        lastInteractedTimestamp[_to] = block.timestamp;
    }

    if (address(transferHookProcessor) != address(0))
        ITransferHookProcessor(transferHookProcessor).afterTokenTransfer(
            _from,
            _to,
            _amount
        );
}

Lines of code

https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/ETHPoolLPFactory.sol#L76

Vulnerability details

Impact

It is possible to rotate LPTokens to a banned BLS public key. This is not a safe action, because it can result in insolvency of the project (specially if the banned BLS public key was malicious).

Proof of Concept

When a user deposits ETH for staking by calling depositETHForStaking, the manager checks whether the provided BLS public key is banned or not. require(liquidStakingNetworkManager.isBLSPublicKeyBanned(_blsPublicKeyOfKnot) == false, "BLS public key is banned or not a part of LSD network"); https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/StakingFundsVault.sol#L113

If it is not banned the LPToken related to that BLS public key will be minted to the caller, so the number of LPToken related to that BLS public key will be increased. https://github.com/code-423n4/2022-11-stakehouse/blob/39a3a84615725b7b2ce296861352117793e4c853/contracts/liquid-staking/ETHPoolLPFactory.sol#L125

If it is banned, it will not be possible to stake to this BLS public key, so the number of LPToken will not be increased. But the issue is that it is still possible to increase the LPToken of this BLS public key through rotating LPToken.

In other words, a malicious user can call rotateLPTokens, so that the _oldLPToken will be migrated to _newLPToken which is equal to the LPToken related to the banned BLS public key.

In summary, the vulnerability is that during rorating LPTokens, it is not checked that the _newLPToken is related to a banned BLS public key or not.

Tools Used

Recommended Mitigation Steps

The following line should be added to function rotateLPTokens(...): require(liquidStakingNetworkManager.isBLSPublicKeyBanned(blsPublicKeyOfNewKnot ) == false, "BLS public key is banned or not a part of LSD network"); https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/ETHPoolLPFactory.sol#L76

Lines of code

https://github.com/code-423n4/2022-11-stakehouse/blob/39a3a84615725b7b2ce296861352117793e4c853/contracts/liquid-staking/StakingFundsVault.sol#L175 https://github.com/code-423n4/2022-11-stakehouse/blob/39a3a84615725b7b2ce296861352117793e4c853/contracts/liquid-staking/SavETHVault.sol#L127 https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/ETHPoolLPFactory.sol#L80

Vulnerability details

Impact

Whenever a user would like to deposit ETH, there is a check that the deposited amount is higher than MIN_STAKING_AMOUNT. https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/ETHPoolLPFactory.sol#L111

The same applies when withdrawing:

• burnLPForETH in StakingFundsVault https://github.com/code-423n4/2022-11-stakehouse/blob/39a3a84615725b7b2ce296861352117793e4c853/contracts/liquid-staking/StakingFundsVault.sol#L175
• burnLPToken in SavETHVault https://github.com/code-423n4/2022-11-stakehouse/blob/39a3a84615725b7b2ce296861352117793e4c853/contracts/liquid-staking/SavETHVault.sol#L127
• rotateLPTokens in ETHPoolLPFactory https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/ETHPoolLPFactory.sol#L80

If the remaining amount is less than MIN_STAKING_AMOUNT, the user can not withdraw it.

Proof of Concept

Suppose a user deposits 0.002 ETH, since this value is higher than MIN_STAKING_AMOUNT, there will be no issue. https://github.com/code-423n4/2022-11-stakehouse/blob/39a3a84615725b7b2ce296861352117793e4c853/contracts/liquid-staking/StakingFundsVault.sol#L113 https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/ETHPoolLPFactory.sol#L111

Later, the user decides to withdraw 0.0015 ETH by burning the equivalent LP tokens, since this value is higher than MIN_STAKING_AMOUNT, there will be no issue. https://github.com/code-423n4/2022-11-stakehouse/blob/39a3a84615725b7b2ce296861352117793e4c853/contracts/liquid-staking/StakingFundsVault.sol#L175

Later the user again intends to withdraw the remaining 0.0005 ETH, but his time it will not be possible because it is less than MIN_STAKING_AMOUNT.

The vulnerability is that during withdrawing, the protocol does not check that the remaining balance of the user is higher than 0.001 ETH or not.

This can be more complicated/dangerous because a user can transfer LP token to other addresses, and the receiver is not aware that amount less than 0.001 ether can not be withdrawn.

Tools Used

Recommended Mitigation Steps

For example the code in burnLPForETH should be checked as follows: https://github.com/code-423n4/2022-11-stakehouse/blob/39a3a84615725b7b2ce296861352117793e4c853/contracts/liquid-staking/StakingFundsVault.sol#L174

function burnLPForETH(LPToken _lpToken, uint256 _amount) public nonReentrant {

    uint256 remaining = _lpToken.balanceOf(msg.sender) - _amount;
    require(remaining == 0 || remaining >= MIN_STAKING_AMOUNT);

    // Rest of the code
}

Lines of code

https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/LiquidStakingManager.sol#L308 https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/LiquidStakingManager.sol#L289

Vulnerability details

Impact

It is not allowed to add non-EOA representative to the smart wallet. But, this limitation can be bypassed by rotating representatives.

Proof of Concept

During registering a node runner to LSD by creating a new smart wallet, it is checked that the _eoaRepresentative is an EOA or not.

require(!Address.isContract(_eoaRepresentative), "Only EOA representative permitted");

https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/LiquidStakingManager.sol#L426

But this check is missing during rotating EOA representative in two functions rotateEOARepresentative and rotateEOARepresentativeOfNodeRunner.

https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/LiquidStakingManager.sol#L289 https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/LiquidStakingManager.sol#L308

In other words _newRepresentative can be a contract in these two functions without being prevented. So, this can bypass the check during registering a node runner to LSD.

Tools Used

Recommended Mitigation Steps

The following line should be added to functions rotateEOARepresentative and rotateEOARepresentativeOfNodeRunner:

require(!Address.isContract(_newRepresentative), "Only EOA representative permitted");

Lines of code

https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/LiquidStakingManager.sol#L280

Vulnerability details

Impact

DAO is not able to update whitelist status of a node runner.

Proof of Concept

The check for Unnecessary update to same status will always revert.

function updateNodeRunnerWhitelistStatus(address _nodeRunner, bool isWhitelisted) external onlyDAO {
        require(_nodeRunner != address(0), "Zero address");
        require(isNodeRunnerWhitelisted[_nodeRunner] != isNodeRunnerWhitelisted[_nodeRunner], "Unnecessary update to same status");

        isNodeRunnerWhitelisted[_nodeRunner] = isWhitelisted;
        emit NodeRunnerWhitelistingStatusChanged(_nodeRunner, isWhitelisted);
    }

https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/LiquidStakingManager.sol#L280

Tools Used

Recommended Mitigation Steps

The function should be modified as:

function updateNodeRunnerWhitelistStatus(address _nodeRunner, bool isWhitelisted) external onlyDAO {
        require(_nodeRunner != address(0), "Zero address");
        require(isNodeRunnerWhitelisted[_nodeRunner] != isWhitelisted, "Unnecessary update to same status");

        isNodeRunnerWhitelisted[_nodeRunner] = isWhitelisted;
        emit NodeRunnerWhitelistingStatusChanged(_nodeRunner, isWhitelisted);
    }