LSD Network - Stakehouse contest - B2's results

A permissionless 3 pool liquid staking solution for Ethereum.

General Information

Platform: Code4rena

Start Date: 11/11/2022

Pot Size: $90,500 USDC

Total HM: 52

Participants: 92

Period: 7 days

Judge: LSDan

Total Solo HM: 20

Id: 182

League: ETH

Stakehouse Protocol

Findings Distribution

Researcher Performance

Rank: 72/92

Findings: 1

Award: $52.03

QA:

grade-b

🌟 Selected for report: 0

🚀 Solo Findings: 0

External Links

Code4rena Contest Page Github Contest Repository Github Findings Repository Stakehouse Protocol

Findings Information

🌟 Selected for report: 0xSmartContract

Also found by: 0x4non, 0xNazgul, 0xRoxas, 0xdeadbeef0x, 0xmuxyz, 9svR6w, Awesome, Aymen0909, B2, Bnke0x0, CloudX, Deivitto, Diana, Franfran, IllIllI, Josiah, RaymondFam, ReyAdmirado, Rolezn, Sathish9098, Secureverse, SmartSek, Trust, Udsen, a12jmx, aphak5010, brgltd, bulej93, c3phas, ch0bu, chaduke, chrisdior4, clems4ever, cryptostellar5, datapunk, delfin454000, fs0c, gogo, gz627, hl_, immeas, joestakey, lukris02, martin, nogo, oyc_109, pashov, pavankv, peanuts, pedr02b2, rbserver, rotcivegaf, sahar, sakman, shark, tnevler, trustindistrust, zaskoh, zgo

Awards

52.0338 USDC - $52.03

Labels

bug

grade-b

QA (Quality Assurance)

Q-29

External Links

Link to GitHub issue

Missing checks for `address(0x0)` when assigning values to `address` state variables

Missing checks for zero-addresses may lead to infunctional protocol, if the variable addresses are updated incorrectly.

        liquidStakingManager = ILiquidStakingManager(_manager);

File: contracts/liquid-staking/OptionalHouseGatekeeper.sol (line 15)

        pool = _pool;
        transferHookProcessor = ITransferHookProcessor(_transferHookProcessor);

File: contracts/liquid-staking/GiantLP.sol (line 25-26)

	Other instances of this issue are:

https://github.com/code-423n4/2022-11-stakehouse/blob/main/contracts/syndicate/SyndicateFactory.sol#L17

Upgradeable contract is missing a `__gap[50]` storage variable to allow for new storage variables in later versions

While some contracts may not currently be sub-classed, adding the variable now protects against forgetting to add it in the future.

        contract LPToken is ILPTokenInit, ILiquidStakingManagerChildContract, Initializable, ERC20PermitUpgradeable {

File: contracts/liquid-staking/LPToken.sol (line 11)

        contract LiquidStakingManager is ILiquidStakingManager, Initializable, ReentrancyGuard, StakehouseAPI {

File:contracts/liquid-staking/LiquidStakingManager.sol (line 33)

	Other instances of this issue are:

open `TODO` comments

Code architecture, incentives, and error handling/reporting questions/issues should be resolved before deployment.

            // todo - check else case for any ETH lost

File: contracts/syndicate/Syndicate.sol (line 195)

Use of `block.timestamp`

Block timestamps have historically been used for a variety of applications, such as entropy for random numbers, locking funds for periods of time, and various state-changing conditional statements that are time-dependent. Miners have the ability to adjust timestamps slightly, which can prove to be dangerous if block timestamps are used incorrectly in smart contracts.

        lastInteractedTimestamp[_from] = block.timestamp;
        lastInteractedTimestamp[_to] = block.timestamp;

File: contracts/liquid-staking/GiantLP.sol (line 44-45)

        lastInteractedTimestamp[_from] = block.timestamp;
        lastInteractedTimestamp[_to] = block.timestamp;

File: contracts/liquid-staking/LPToken.sol (line 76-68)

	Other instances of this issue are:

Unused `receive()` function will lock Ether in contract

If the intention is for the Ether to be used, the function should call another function, otherwise it should revert

    receive() external payable {}

File: contracts/liquid-staking/SyndicateRewardsProcessor.sol (line 98)

    receive() external payable {

File: contracts/smart-wallet/OwnableSmartWallet.sol (line 148)

	Other instances of this issue are:

Set `garbage` value in `mapping` for deleting that

If there is a mapping data structure present inside struct, then deleting the struct doesn't delete the mapping. Instead one should use lock to lock that data structure from further use.

            delete smartWalletRepresentative[_smartWallet];

File: /contracts/liquid-staking/LiquidStakingManager.so (line 713)

	Other instances of this issue are:

Event is missing `indexed` fields

Each event should use three indexed fields if there are three or more fields.

    event LPTokenBurnt(bytes blsPublicKeyOfKnot, address token, address depositor, uint256 amount);

File: contracts/liquid-staking/ETHPoolLPFactory.sol (line 19)

    event ETHWithdrawnForStaking(address withdrawalAddress, address liquidStakingManager, uint256 amount);

File: contracts/liquid-staking/SavETHVault.sol (line 22)

	Other instances of this issue are:

TYPOS

    ///@audit: `determins ` 
    /// @notice Utility function that determins whether an LP can be burned for dETH if the associated derivatives have been minted

File: contracts/liquid-staking/SavETHVault.sol (line 227)

    ///@audit: `admiting ` 
    /// @notice address of optional gatekeeper for admiting new knots to the house created by the network

File: contracts/liquid-staking/LiquidStakingManager.sol (line 101)

	Other instances of this issue are:

https://github.com/code-423n4/2022-11-stakehouse/blob/main/contracts/liquid-staking/LiquidStakingManager.sol#L476 ///@audit: validtor & initals
https://github.com/code-423n4/2022-11-stakehouse/blob/main/contracts/liquid-staking/LiquidStakingManager.sol#L903 ///@audit: overriden
https://github.com/code-423n4/2022-11-stakehouse/blob/main/contracts/liquid-staking/LiquidStakingManager.sol#L920 ///@audit: overriden
https://github.com/code-423n4/2022-11-stakehouse/blob/main/contracts/liquid-staking/ETHPoolLPFactory.sol#L74 ///@audit: Instane

Consider ordering multiplication first.

Solidity could truncate the results, performing multiplication before division will prevent rounding/truncation in solidity math.

                        balance * unprocessedForKnot / (4 ether - currentSlashedAmount);

File: contracts/syndicate/Syndicate.sol (line 431)

                            balance * unprocessedETHForCurrentKnot / (4 ether - currentSlashedAmount);

File: contracts/syndicate/Syndicate.sol (line 522)

`public` functions not called by the contract should be declared `external` instead

Contracts are allowed to override their parents’ functions and change the visibility from external to public.

    function withdrawETH(address _wallet, uint256 _amount) public onlyManager nonReentrant returns (uint256) {

File: contracts/liquid-staking/StakingFundsVault.sol (line 239)

    function withdrawETHForStaking(
        address _smartWallet,
        uint256 _amount
    ) public onlyManager nonReentrant returns (uint256) {

File: contracts/liquid-staking/SavETHVault.sol (line 200-203)

	Other instances of this issue are:

`NatSpec` is incomplete

    /// @audit Missing: '@return`
    /// @param Missing: '@param' "_deployOptionalHouseGatekeeper" & "_optionalCommission"
    /// @notice Deploys a new LSDN and the liquid staking manger required to manage the network
    /// @param _dao Address of the entity that will govern the liquid staking network
    /// @param _stakehouseTicker Liquid staking derivative network ticker (between 3-5 chars)
    function deployNewLiquidStakingDerivativeNetwork(

File: contracts/liquid-staking/LSDNFactory.sol (line 70-73)

    /// @param Missing: '@param 
    /// @notice Mints a given amount of LP tokens
    /// @dev Only savETH vault can mint
    function mint(address _recipient, uint256 _amount) external onlyDeployer {

File: contracts/liquid-staking/LPToken.sol (line 44-46)

	Other instances of this issue are:

https://github.com/code-423n4/2022-11-stakehouse/blob/main/contracts/liquid-staking/GiantPoolBase.sol#L33-L34 /// @param Missing: '@param
https://github.com/code-423n4/2022-11-stakehouse/blob/main/contracts/liquid-staking/LPTokenFactory.sol#L24-L27 /// @audit Missing: '@return` & /// @param Missing: '@param
https://github.com/code-423n4/2022-11-stakehouse/blob/main/contracts/liquid-staking/GiantMevAndFeesPool.sol#L200-L201ok /// @param Missing: '@param
https://github.com/code-423n4/2022-11-stakehouse/blob/main/contracts/liquid-staking/GiantMevAndFeesPool.sol#L55-L56 /// @param Missing: '@param
https://github.com/code-423n4/2022-11-stakehouse/blob/main/contracts/liquid-staking/SavETHVault.sol#L217-L228 /// @audit Missing: '@return` & /// @param Missing: '@param

#0 - c4-judge
2022-12-02T17:45:14Z

dmvt marked the issue as grade-b

LSD Network - Stakehouse contest - B2's results

A permissionless 3 pool liquid staking solution for Ethereum.

General Information

Findings Distribution

Researcher Performance

External Links

[Q-29] QA Report - $52.03

Findings Information

Awards

Labels

External Links

Missing checks for address(0x0) when assigning values to address state variables

Upgradeable contract is missing a __gap[50] storage variable to allow for new storage variables in later versions

open TODO comments

Use of block.timestamp

Unused receive() function will lock Ether in contract

Set garbage value in mapping for deleting that

Event is missing indexed fields

TYPOS

Consider ordering multiplication first.

public functions not called by the contract should be declared external instead

NatSpec is incomplete

#0 - c4-judge2022-12-02T17:45:14Z

Missing checks for `address(0x0)` when assigning values to `address` state variables

Upgradeable contract is missing a `__gap[50]` storage variable to allow for new storage variables in later versions

open `TODO` comments

Use of `block.timestamp`

Unused `receive()` function will lock Ether in contract

Set `garbage` value in `mapping` for deleting that

Event is missing `indexed` fields

`public` functions not called by the contract should be declared `external` instead

`NatSpec` is incomplete

#0 - c4-judge
2022-12-02T17:45:14Z