Remaining amount of yeildFeeBalance will be lost if the yeildFeeRecipient tries to withdraw partially. #62
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
upgraded by judge
Original issue severity upgraded from QA/Gas by judge
Lines of code
https://github.com/code-423n4/2024-03-pooltogether/blob/480d58b9e8611c13587f28811864aea138a0021a/pt-v5-vault/src/PrizeVault.sol#L611-L622
Vulnerability details
Impact
When the
yeildFeeRecipient
tries to claim some amount ofyeildFeeBalance
by calling theclaimYieldFeeShares
function it will mint the given amount of shares but the remainingyeildFeeBalance
will also be deducted.Which result in loss of yeildFees for the yeildFeeRecipient.
Proof of Concept
If we look at the function it takes an argument
uint256 _shares
so it is clear that the caller(yeildFeeRecipient
) has the choice to specify the amount ofyieldFeeBalance
he wants to claim.However regardless of the given amount of
_shares
to claim, the function mints the given amount of_shares
but will always deduct the full amount ofyieldFeeBalance
.It mints the given amount of shares but always deducts the full amount of
yieldFeeBalance
.Since the underlying asset is not withdrawn and still in the yeildVault the assets are not directly lost. It goes back to the availableYeildBalance which will go to the pricePool. There might not be direct loss of assets but the
yeildFeeRecipient
clearly lost his collectible fees.Tools Used
manual.
Recommended Mitigation Steps
Replace this line
yieldFeeBalance -= _yieldFeeBalance;
from theclaimYieldFeeShares
function with this :yieldFeeBalance -= _shares
.Or another approach would be to remove the parameter
uint256 _shares
and always mint the total amount of fees;remove
if (_shares > _yieldFeeBalance) revert SharesExceedsYieldFeeBalance(_shares, _yieldFeeBalance);
and change
_mint(msg.sender, _shares);
to_mint(msg.sender, _yieldFeeBalance);
Assessed type
Other
The text was updated successfully, but these errors were encountered: