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

claimYieldFeeShares decrements yieldFeeBalance by wrong amount #320

Closed
c4-bot-3 opened this issue Mar 11, 2024 · 4 comments
Closed

claimYieldFeeShares decrements yieldFeeBalance by wrong amount #320

c4-bot-3 opened this issue Mar 11, 2024 · 4 comments
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-59 🤖_10_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality

Comments

@c4-bot-3
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2024-03-pooltogether/blob/480d58b9e8611c13587f28811864aea138a0021a/pt-v5-vault/src/PrizeVault.sol#L611-L622

Vulnerability details

Impact

In claimYieldFeeShares the fee recipient can choose the amount of shares that they want to claim from the yieldFeeBalance. The issue is that currently, even if the fee recipient chooses a number less than the entire yieldFeeBalance, yieldFeeBalance is reset to 0, preventing the fee recipient from claiming any more of the fees they were initially granted.
For example, if the fee recipient chooses to claim only 50 of 100 from the yieldFeeBalance, yieldFeeBalance will be reset to 0, and the fee recipient will not be able to claim the remaining 50 yield fees.

Proof of Concept

As we can see yieldFeeBalance is decremented by _yieldFeeBalance instead of _shares.

https://github.com/code-423n4/2024-03-pooltogether/blob/480d58b9e8611c13587f28811864aea138a0021a/pt-v5-vault/src/PrizeVault.sol#L617C13-L617C45

   yieldFeeBalance -= _yieldFeeBalance;

Tools Used

Manual review

Recommended Mitigation Steps

Update from:

https://github.com/code-423n4/2024-03-pooltogether/blob/480d58b9e8611c13587f28811864aea138a0021a/pt-v5-vault/src/PrizeVault.sol#L617C13-L617C45

   yieldFeeBalance -= _yieldFeeBalance;

To:

   yieldFeeBalance -= _shares;

Assessed type

Other

@c4-bot-3 c4-bot-3 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Mar 11, 2024
c4-bot-10 added a commit that referenced this issue Mar 11, 2024
@c4-bot-12 c4-bot-12 added the 🤖_10_group AI based duplicate group recommendation label Mar 11, 2024
@c4-pre-sort c4-pre-sort added the sufficient quality report This report is of sufficient quality label Mar 11, 2024
@c4-pre-sort
Copy link

raymondfam marked the issue as sufficient quality report

@c4-pre-sort
Copy link

raymondfam marked the issue as duplicate of #10

@c4-pre-sort
Copy link

raymondfam marked the issue as duplicate of #59

@c4-judge
Copy link
Contributor

hansfriese marked the issue as satisfactory

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Mar 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate-59 🤖_10_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

4 participants