Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

0x73696d616f - VaultPoolLib::reserve() will store the Pa not attributed to user withdrawals incorrectly and leave in untracked once it expires again #191

Open
sherlock-admin3 opened this issue Sep 10, 2024 · 2 comments
Labels
Has Duplicates A valid issue with 1+ other issues describing the same vulnerability High A High severity issue. Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Won't Fix The sponsor confirmed this issue will not be fixed

Comments

@sherlock-admin3
Copy link

sherlock-admin3 commented Sep 10, 2024

0x73696d616f

High

VaultPoolLib::reserve() will store the Pa not attributed to user withdrawals incorrectly and leave in untracked once it expires again

Summary

VaultPoolLib::reserve() stores the Pa attributed to withdrawals in self.withdrawalPool.stagnatedPaBalance instead of storing the amount attributedToAmm. Additionally, this amount of Pa, the one attributed to the Amm is never dealt with and leads to stuck PA.

The comment in the code mentions

    // FIXME : this is only temporary, for now
    // we trate PA the same as RA, thus we also separate PA
    // the difference is the PA here isn't being used as anything
    // and for now will just sit there until rationed again at next expiry.

But it is incorrect as it is never rationed again, just forgotten. The VaultPoolLib::rationedToAmm() function only uses the Ra balance, not the Pa, which is effectively left untracked.

Root Cause

In VaultPoolLib:170, the leftover non attributed Pa is not dealt with.

Internal pre-conditions

None.

External pre-conditions

None.

Attack Path

  1. VaultPoolLib::reserve() is called when liquidating the lp position of the Vault via VaultLib::_liquidatedLP(), triggered by users when redeeming expired liquidity vault shares or on the admin trigerring a new issuance.

Impact

The Pa in the Vault is stuck.

PoC

VaultPoolLib::rationedToAmm() does not deal with the Pa.

function rationedToAmm(VaultPool storage self, uint256 ratio) internal view returns (uint256 ra, uint256 ct) {
    uint256 amount = self.ammLiquidityPool.balance;

    (ra, ct) = MathHelper.calculateProvideLiquidityAmountBasedOnCtPrice(amount, ratio);
}

Mitigation

Distributed the Pa to users based on their LV shares or redeem the Pa for Ra and add liquidity to the new issued Ds or similar.

@ziankork
Copy link
Collaborator

valid. will fix

@sherlock-admin3 sherlock-admin3 added Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed labels Sep 24, 2024
@sherlock-admin3 sherlock-admin3 changed the title Blurry Blush Mouse - VaultPoolLib::reserve() will store the Pa not attributed to user withdrawals incorrectly and leave in untracked once it expires again 0x73696d616f - VaultPoolLib::reserve() will store the Pa not attributed to user withdrawals incorrectly and leave in untracked once it expires again Sep 25, 2024
@sherlock-admin3 sherlock-admin3 added the Reward A payout will be made for this issue label Sep 25, 2024
@ziankork
Copy link
Collaborator

ziankork commented Oct 4, 2024

This is originally has a feature planned for that, but we're still working on it and the feature still needs some time for us to solidify all aspect of it. that's why we won't fix this. Since the feature are considered non-trivial

@sherlock-audit sherlock-audit deleted a comment from sherlock-admin2 Oct 4, 2024
@sherlock-admin3 sherlock-admin3 added Won't Fix The sponsor confirmed this issue will not be fixed and removed Will Fix The sponsor confirmed this issue will be fixed labels Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Has Duplicates A valid issue with 1+ other issues describing the same vulnerability High A High severity issue. Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Won't Fix The sponsor confirmed this issue will not be fixed
Projects
None yet
Development

No branches or pull requests

2 participants