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

4 stars 4 forks source link

cats - Minting formula does not subtract pending withdrawals #354

Closed sherlock-admin2 closed 8 months ago

sherlock-admin2 commented 8 months ago

cats

high

Minting formula does not subtract pending withdrawals

Summary

The minting formula in the coordinator does not take into account pending withdrawals when calculating minted shares.

Vulnerability Detail

Deposits in the protocol are atomic, users transfer in tokens and are minted shares. Withdrawals on the other hand, are not atomic and are submitted as a pending request.

    withdrawalQueue().queueWithdrawal(msg.sender, asset, sharesOwed, amountIn);

Due to this difference, an issue arises when users deposit their funds and are minted shares based on the deposit pool balance.

Since withdrawal requests cannot be cancelled and sit in the deposit pool while waiting to be paid out, the deposit function will mint shares inaccurately based on funds that are queued for withdrawal, instead of assetTVL - withdrawalRequests[]. Once per 24h a rebalance can be attempted for that asset, and if the assets are paid out to all the requests, the share value will drop significantly.

Impact

Wrong calculation when minting negatively impacts share valuation of depositing users and leads to value loss.

Code Snippet

    withdrawalQueue().queueWithdrawal(msg.sender, asset, sharesOwed, amountIn);

Tool used

Manual Review

Recommendation

Calculate shares minted from assetTVL - withdrawalRequests[] since withdrawals cannot be cancelled and there's no risk of front/backrunning price manipulation and griefing attacks.

Duplicate of #361

solimander commented 8 months ago

Can you POC this one? I'm not understanding how the current flow "negatively impacts share valuation of depositing users and leads to value loss" as we're not burning LRT at the time of request.