code-423n4 / 2024-03-pooltogether-findings

5 stars 4 forks source link

Loss of Funds Due to Incorrect Deduction in `claimYieldFeeShares` Function #272

Closed c4-bot-2 closed 6 months ago

c4-bot-2 commented 6 months ago

Lines of code

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

Vulnerability details

Issue Description

The claimYieldFeeShares function in the contract is responsible for allowing the yield fee recipient to claim their shares of the yield fee. However, a critical issue has been identified in the deduction logic within the function:

function claimYieldFeeShares(uint256 _shares) external onlyYieldFeeRecipient {
    if (_shares == 0) revert MintZeroShares();

    uint256 _yieldFeeBalance = yieldFeeBalance;
    if (_shares > _yieldFeeBalance) revert SharesExceedsYieldFeeBalance(_shares, _yieldFeeBalance);

    //@audit will decrease all fee balance regardless of amount of shares claimed
    yieldFeeBalance -= _yieldFeeBalance;

    _mint(msg.sender, _shares);

    emit ClaimYieldFeeShares(msg.sender, _shares);
}

As it can be seen the function decrease the yield fee balance (yieldFeeBalance) not based on the _shares parameter provided to the function, but instead the entire balance (_yieldFeeBalance) is deducted, which can result in a loss of funds for the fee recipient because the remaining yield shares (yieldFeeBalance - _shares) will be removed and the recipient will never be able to mint them again.

Impact

The incorrect deduction logic in the claimYieldFeeShares function can lead to a significant loss of funds for the yield fee recipient.

Tools Used

Manual review, VS Code

Recommended Mitigation

Review and correct the deduction logic within the claimYieldFeeShares function to ensure that only the appropriate amount of yield fee is deducted based on the _shares parameter provided.

function claimYieldFeeShares(uint256 _shares) external onlyYieldFeeRecipient {
    if (_shares == 0) revert MintZeroShares();

    uint256 _yieldFeeBalance = yieldFeeBalance;
    if (_shares > _yieldFeeBalance) revert SharesExceedsYieldFeeBalance(_shares, _yieldFeeBalance);

--  yieldFeeBalance -= _yieldFeeBalance;
++  yieldFeeBalance -= _shares;

    _mint(msg.sender, _shares);

    emit ClaimYieldFeeShares(msg.sender, _shares);
}

Assessed type

Error

c4-pre-sort commented 6 months ago

raymondfam marked the issue as sufficient quality report

c4-pre-sort commented 6 months ago

raymondfam marked the issue as duplicate of #10

c4-pre-sort commented 6 months ago

raymondfam marked the issue as duplicate of #59

c4-judge commented 6 months ago

hansfriese marked the issue as satisfactory