LSD Network - Stakehouse contest - sahar'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: 49/92

Findings: 2

Award: $96.32

QA:
grade-b

🌟 Selected for report: 0

πŸš€ Solo Findings: 0

Findings Information

🌟 Selected for report: yixxas

Also found by: cccz, joestakey, pashov, sahar

Labels

2 (Med Risk)
partial-50
duplicate-190

Awards

44.2926 USDC - $44.29

External Links

Judge has assessed an item in Issue #236 as M risk. The relevant finding follows:

https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/liquid-staking/LiquidStakingManager.sol#L249 It is possible the DAO determine the amount of commission very high for its own benefit, so setting a range for determining the commission seems logical. (Especially MAX commission should be pre- defined.)

#0 - c4-judge

2022-12-01T23:44:51Z

dmvt marked the issue as duplicate of #190

#1 - c4-judge

2022-12-02T17:19:25Z

dmvt marked the issue as partial-50

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

This function need a modifier or require, So only authorized address (giant LP l) can call it.


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

Using β€œ>=” instead of β€œ==” in this command is more logical. In some cases, maybe the user can't calculate an exact amount.


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

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

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

Smaller variable can be used.


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

Changing this important role should be done in two steps. First, the new address to get the role should be introduced, and then the new address claims the role.


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

It is possible the DAO determine the amount of commission very high for its own benefit, so setting a range for determining the commission seems logical. (Especially MAX commission should be pre- defined.)


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

It may be necessary to change the address of the deployer in this contract in the future for any reason. This is an important role and there is a possibility of needing to change it. Therefore, in this agreement, there should be a two-step function where the current deployer can introduce another address as a new deployer role. Then new address call a function and get role.


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

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

Unlike the workflow of withdrawing Ether function from a contract, in this deposit function , it makes sense to add a new amount to the total supply just after a successful deposit operation.
In other words, it is better to swap place of lines 71 and 72 together.


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

In this file, the pragma version is only set to β€œ0.8.13” , while in other files of this project, it is set to β€œ^0.8.13” ( this version and above till 0.9.0). This version difference may cause an unknown problem during the compilation and deployment of the main contract on the mainnet.


https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/smart-wallet/OwnableSmartWalletFactory.sol#L36

ٌWallet may exist before calling this function (already cloned). Although calling and re-creating the wallet does not cause any problem, but by checking the condition [ if walletExists[wallet] == true]b before re-clone, you can avoid the re-execution of the function body and wasteful consumption of gas.


https://github.com/code-423n4/2022-11-stakehouse/blob/4b6828e9c807f2f7c569e6d721ca1289f7cf7112/contracts/smart-wallet/OwnableSmartWallet.sol#L132

You can check the following condition [if (_isTransferApproved[from][to] != status)] and execute the next assignment command only if the condition is true, thus avoiding unnecessary gas consumptioan in some cases. For this, it is enough to swap the two lines number 132 and 133.


Thank you πŸ™‚

#0 - c4-judge

2022-12-01T23:44:29Z

dmvt marked the issue as grade-b

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax Β© 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter