hats-finance / Kintsu-0x7d70f9442af3a9a0a734fa6a1b4857f25518e9d2

Smart contracts for Kintsu
Other
0 stars 0 forks source link

rewards will be stuck in the nomination_agent #66

Open hats-bug-reporter[bot] opened 5 months ago

hats-bug-reporter[bot] commented 5 months ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0xe7feeede30067fd9df475404fa48c38bee5103b4d9d464940b89d4f88c08326e Severity: high

Description: Description\ While nomination_agent.deposit is called, the function will fall into calling BondExtra if self.staked is larger than 0.

And then nomination-pools.bond_extra will be called.

And nomination-pools.do_bond_extra will be called in nomination-pools.bond_extra. In nomination-pools.do_bond_extra, do_reward_payout will be called in lib.rs#L3188. According to nomination_agent.do_reward_payout, the function is used to transfer reward to nomination_agent

And back to nomination_agent.deposit, the function doesn't handle the received rewards.

NOTE, similar issue also exists in nomination-pools.unbond

Attack Scenario\ Describe how the vulnerability can be exploited. Please consider the following case:

  1. Alice calls vault.stake to deposit some AZERO
  2. Alice calls vault.stake to deposit more AZERO
  3. During the second stake call, the nomination-pools will send some rewards to nomination_agent, but nomination_agent doesn't handle it.

The issue is high severity because everytime a user calls vault.stake/start_unbond, there will be some reward stuck in nomination_agent

coreggon11 commented 5 months ago

I believe this is handled by compound.

crazy4linux commented 5 months ago

By using compound, it means that the asset will rebond to nomination-pools. I think there is chance that nomination_agent does deposit -> unbond -> withdraw

crazy4linux commented 5 months ago

Maybe my description isn't clear enough.

  1. Alice calls vault.stake to deposit some AZERO
  2. Alice calls vault.stake to deposit more AZERO
  3. During the second stake call, the nomination-pools will send some rewards to nomination_agent, but nomination_agent doesn't handle it.
  4. Even the reward can be rebond to the pool, but Alice can't receive her rewards by during the stake. And some other person who stake later will get more reward than expected.
coreggon11 commented 5 months ago

From my understanding, the reward is handled by the compound function, so Alice can call compound, and the contract will handle the reward. Alice would not be able to "get" her reward anyway, since she needs to first request withdrawal etc.

0xmahdirostami commented 5 months ago

@coreggon11 is right, there is no issue here.

The issue is high severity because everytime a user calls vault.stake/start_unbond, there will be some reward stuck in nomination_agent

You are right, changing the bonded amount (stake or unbond) will send rewards to the agent, but there is no issue here. Withdrawal is designed to just send balance before and after. Compound is designed to use the whole balance..

0xmahdirostami commented 5 months ago

By using compound, it means that the asset will rebond to nomination-pools. I think there is chance that nomination_agent does deposit -> unbond -> withdraw

what is the issue here?

0xmahdirostami commented 5 months ago

There is no access control or restriction on the compound function. Users can call the compound function whenever they want. In addition, there is an incentive mechanism for the compound function. If there are considerable rewards, bots will call the function.

crazy4linux commented 5 months ago

@0xmahdirostami @bgibers Even bot calls compound, I still think there will be issue.

Please consider the following case:

  1. Alice is the only user in the system
  2. Alice calls vault.stake to deposit 100e12 AZERO, vault will mint Alice 100E12 sAZERO, and nomination-pools will issue nomination_agent 100e12 points according to nomination-pools#L1273
  3. Alice calls unlock_request and delegate_withdraw_unbonded to redeem all her sAZERO, which is 100e12, during start_unbond, there will be some rewards sent to the nomination_agent by nomination-pools, and suppose the rewards is 15e12 AZERO
  4. compound is called by someone, the 15e12 AZERO is sent to nomination-pools, and nomination_agent will get another 15e12 points
  5. Because Alice only owns 100e12 sAZERO, which is 100e12 points, she can't withdraw the extra rewards(which is 15e12 AZERO)
crazy4linux commented 5 months ago

And regarding to compound, as https://github.com/hats-finance/Kintsu-0x7d70f9442af3a9a0a734fa6a1b4857f25518e9d2/issues/30 stats, it can be DOSed

bgibers commented 5 months ago

@crazy4linux I think you might be a bit confused on what compound is doing still. The call is fully intended to rebond the reward tokens. delegateWithdrawUnbonded returns all gas tokens that are unbonded back to the vault including accrued rewards.

per Unstaking Order of Operations:

Token redemption: This is where the user can redeem their Gas Tokens, including accrued yield. In order for this to happen, the Vault must have custody of the unbonded Gas Tokens. There are two scenarios here:

If the newly unbonded Gas Tokens are still held in Nomination Pools when a user tries to redeem theirs, the user must call redeemWithWithdraw. This first executes a delegateWithdrawUnbonded call, during which the Vault collects the unbonded Gas Tokens from each of the Nomination Pools, and then executes the redeem call, which forwards the user's entitled portion of those Gas Tokens from the Vault to the user's wallet.

If the Vault already has custody of the Gas Tokens, the user can make a redeem call to the Vault, which sends the user's newly unstaked Gas Tokens to their wallet. This scenario would happen when someone else has already executed the delegateWithdrawUnbonded call for the Batching Period.

crazy4linux commented 5 months ago

I see, thanks for explaintion