Open hats-bug-reporter[bot] opened 4 days ago
The share could not be decreased fairly without the use of any oracle. Additionally, the share divided by the total supply represents the user's share in the portfolio. It's the asset manager's responsibility to claim the tokens. We'll take snapshots of the current balances when a user receives or burns tokens to ensure fair distribution of the removed tokens. @burhankhaja
At this point, i believe there is something wrong with my english, why are u not getting my point. i am talking about the tokens held by portfolio.sol contract outside of the portfolio tokens can be claimed by users which should be considered bug,
There is no way asset manager can claim them, until he himself owns 100% of the portfolio(as an investor) which is impossible for the portfolio management business.
Common, give it a thought it is visible bug Have u properly understood the attack scenario part?
I think we need to understand how and why the non-portfolio token exist in the vault. It is not just a random person send the non-portfolio token to the vault, there is no reason to do that.
IMO, this non-portfolio token came from claimRewardTokens
so the _tokenToBeClaimed
might be the non-portfolio token. It's clear if this reward token is the non-portfolio asset manager can remove them from vault to TokenExclusiveManager
for equal share distribution. maybe @langnavina97 can confirm this.
@burhankhaja reading your comments on your submissions, you seems frustrated, please calm a bit brother. Recalling you 100% sure a high issue and eagerly want to create POC on #93 , but IMO, you are not understand the protocol well on that issue.
Correct @kakarottosama !
A non-portfolio token can be the result from claiming reward tokens. Asset managers will have the choice to either rebalance the tokens (reinvest) or remove the tokens so the portfolio token holders can claim them separately. Taking the snapshot data ensures a fair distribution of the claimed tokens.
@burhankhaja
Github username: @burhankhaja Twitter username: imaybeghost Submission hash (on-chain): 0xda18344374bba1a2eed7576a315ad463e74c004194e4f8fc091b85853469111c Severity: high
Description: Description\ As Per the natspec, The Purpose of claimRemovedTokens() is to Allows users to claim their share of tokens that have been removed from the portfolio due to certain events (e.g., lack of liquidity).
When the AssetManager removes portfolio tokens, they are sent to the TokenExclusiveManager to be claimed by the user.
Everything about the calculation is correct and works as expected in case of portfolio token removals. The user is able to claim removed portfolio token as per his shares (portfoliotoken balances) and no unexpected fund loss occurs.
But in case of non-portfolio token removals,
User shouldn't be able to claim them, or his shares should get subtracted, since he didn't contribute anything to the non-portfolio tokens of the vault contract. (Portfolio.sol)
Attack Scenario\ Imagine this case:
ALICE
owns 25% of the portfolioALICE
will be able to claim 25 ETH via claimRemovedTokens as the same logic is applied for both portfolio and non-portfolio token removals (where the user's percentage of portfoliotokens is calculated, since alice owns 25% of portfolio tokens, therefore he will receive 25 ETH)Recommendation\ Either separate the logic for Portfolio and Non-Portfolio token removals or decrease the user's share balances while claiming non-portfolio tokens.
Attachments
Proof of Concept (PoC) File
Revised Code File (Optional)