hats-finance / StakeWise-0xd91cd6ed6c9a112fdc112b1a3c66e47697f522cd

Liquid staking protocol for Ethereum
Other
0 stars 0 forks source link

staking fee could be changed #129

Open hats-bug-reporter[bot] opened 1 year ago

hats-bug-reporter[bot] commented 1 year ago

Github username: @0xmahdirostami Submission hash (on-chain): 0xf0acf9cfbd693aa934533c3708b3ec314b738c345ee9518ba8171039e01db397 Severity: high

Description: Description\ The staking fee is an important parameter and StakeWise promises that this parameter couldn't change.

Note that the Vault Admin cannot change the staking fee once the Vault is registered, so stakers can have peace of mind about staking with Vaults.

and there is no function for changing it, but the admin could upgrade the vault and change the fee.

Impact\

Attachments

  1. Proof of Concept (PoC) File

    @@ -20,8 +20,11 @@ contract EthVaultV2Mock is EthVault {
    {}
    
    function initialize(bytes calldata data) external payable virtual override reinitializer(2) {
    -    (newVar) = abi.decode(data, (uint128));
    +    (feePercent) = abi.decode(data, (uint16));
    +    __VaultFee_init(msg.sender, feePercent);
    }

    Test:

    describe('EthVault - upgrade', () => {
    const capacity = parseEther('1000')
    -  const feePercent = 1000
    +  const feePercent = 50   //first fee
    const metadataIpfsHash = 'bafkreidivzimqfqtoqxkrpge6bjyhlvxqs3rhe73owtmdulaxr5do5in7r'
    let admin: Wallet, dao: Wallet, other: Wallet
    let vault: EthVault
    
     currImpl = await vault.implementation()
    -    callData = defaultAbiCoder.encode(['uint128'], [100])
    +    callData = defaultAbiCoder.encode(['uint16'], [100])
     await vaultsRegistry.connect(dao).addVaultImpl(newImpl)
     updatedVault = (await ethVaultMock.attach(vault.address)) as EthVaultV2Mock
    
    it('works with valid call data', async () => {
    +    const before_fee = await vault.feePercent()
    +    console.log('fee in the first version:', before_fee)
     const receipt = await vault.connect(admin).upgradeToAndCall(newImpl, callData)
    +    const after_fee = await vault.feePercent()
    +    console.log('fee in the second version:', after_fee)

    Log

    fee in the first version: 50
    fee in the second version: 100
  2. Revised Code File (Optional)

    Add a check for not changing the vault fee.

I want to mention that other parameters(capacity, ...) could be changed, and if those are important add a check for them.

tsudmi commented 1 year ago

The admin cannot update the vault by himself, the new implementation contract requires approval from the DAO.