sherlock-audit / 2024-02-rio-network-core-protocol-judging

4 stars 4 forks source link

HSP - Unallocated assets may largely reduce the profit belongs to the user whose funds are utilized #202

Closed sherlock-admin2 closed 6 months ago

sherlock-admin2 commented 7 months ago

HSP

medium

Unallocated assets may largely reduce the profit belongs to the user whose funds are utilized

Summary

Unallocated assets may largely reduce the profit belongs to the user whose funds are utilized.

Vulnerability Detail

When user deposits to Rio, LRT tokens are first minted to user to the user, and when rebalancing, allocateStrategyShares(...) function is called to allocate shares to the operators.If the allocation of the operator with the lowest utilization rate is maxed out, function exits earlier and no more shares will be allocated.

            if (operatorShares.allocation >= operatorShares.cap) break;

The unallocated assets are not returned to user but stay in the deposit pool. This is problematic because the rewards protocol gains from Eigenlayer are shared by the LRT holders, given LRT tokens are also minted for the unallocated assets, User's profit can be largely reduced.

Impact

User's profit is largely reduced.

Code Snippet

https://github.com/sherlock-audit/2024-02-rio-network-core-protocol/blob/main/rio-sherlock-audit/contracts/restaking/RioLRTOperatorRegistry.sol#L342

Tool used

Manual Review

Recommendation

Return funds back to user if reaches the cap, and only mint LRT tokens to users whose funds are utilized.

nevillehuang commented 7 months ago

request poc

sherlock-admin3 commented 7 months ago

PoC requested from @0xhsp

Requests remaining: 7

0xhsp commented 7 months ago

It's hard to write a POC but allow me to elaborate:

  1. Rio network receives rewards from EigenLayer only if protocol stakes ETH/LST tokens on EigenLayer, and the rewards is based on the protocol's staking amount;
  2. Rio user receives rewards from Rio Network if only the user holds LRT tokens, and user is minted LRT tokens when he/she deposits;
  3. However the ETH/LST tokens user deposits may not be staked on EigenLayer through the protocol hence yield no rewards.

For example:

  1. 10 user deposit 1000 LST tokens and the tokens are staked on EigenLayer, protocol receives 10 ETH rewards per day, so each user receives 1 ETH from Rio.
  2. If 10 more users deposit 1000 more LST tokens but due to the operators cap, the tokens are not staked on EigenLayer, protocol still receives 10 ETH rewards per day from EigenLayer but each user only receives 0.5 ETH.

Besides, attacker may avoid slashing by front-run EigenLayer slashing transaction to call requestWithdraw() and rebalance() (this is possible because rebalance() may not have been called because there is no deposits/withdrawals), then the un-unitized funds in deposit pool is withdrawn to attacker, as a result, attacker earn riskless profit while the other users suffer more loss due to the slashing.

So I believe the unallocated assets is a problem, the funds should be transferred back to users and the LRT should be minted/burned accordingly.

solimander commented 7 months ago

This seems to be a management issue, not a code issue. The deposit and operator caps will be managed in such a way that yield drag is reduced.