code-423n4 / 2022-06-yieldy-findings

0 stars 0 forks source link

Fund migration should trigger a rebase to prevent missing out on potential rewards #276

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-yieldy/blob/524f3b83522125fb7d4677fa7a7e5ba5a2c0fe67/src/contracts/Migration.sol#L54

Vulnerability details

Impact

Rebasing allows the protocol to "distribute" profit/rewards to Yieldy (and Foxy) token holders by increasing the supply of tokens and increasing the balance of each token holder relative to the token balance (creditBalances).

The order of rebasing and unstaking matters, as the token balance of an account is calculated based on the current balance (technically it's not the token balance, it's credit balances creditBalances).

Unstaking (reducing the account balance) before a rebase will make a user loose out on the distributed (rebased) rewards.

Hence, it's always advised to rebase before any unstaking event or whenever the creditBalances of an account are reduced.

Impact: Migrating funds from the old Foxy contract to the new Yieldy contracts while having an outstanding rebase (profit was not distributed yet) will lead to a loss of potential rewards from the old staking contract.

Proof of Concept

Migration.sol#L54

IStaking(OLD_CONTRACT).instantUnstake(false); // @audit-info the boolean function parameter determines if a rebase should occur. In this case, no rebase will take place

Tools Used

Manual review

Recommended mitigation steps

Use true as a function parameter for instantUnstake to always rebase and distribute potential rewards.

toshiSat commented 2 years ago

dispute severity: This seems low/medium, but you are correct and we will change this.