sherlock-audit / 2023-05-perennial-judging

12 stars 9 forks source link

bitsurfer - Users are unable to obtain the remaining unclaimed collateral when `totalCollateral` less than `unclaimedTotal` #186

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

bitsurfer

medium

Users are unable to obtain the remaining unclaimed collateral when totalCollateral less than unclaimedTotal

Summary

Users are unable to obtain the remaining unclaimed collateral when totalCollateral less than unclaimedTotal

Vulnerability Detail

Users are unable to obtain the remaining unclaimed collateral when totalCollateral less than unclaimedTotal. This problem arises due to a fail logic within the claim() function. It fails to handle situations where the available collateral is insufficient to cover the entire unclaimed amount. As a result, users receive a reduced collateral payout, and their unclaimed balance is not properly accounted for or carried forward.

Due to the existance of this following line before knowing that user will only get partial claim for their collateral, user will zeroed-out to claim the rest of their collateral.

_unclaimed[account] = UFixed18Lib.ZERO;

The claim() function should ensure that users receive the correct proportion of collateral based on their unclaimed balance, even if the total collateral is lower than the total unclaimed amount. Additionally, any remaining unclaimed collateral should be properly stored and made accessible to users for future claims.

File: BalancedVault.sol
211:     function claim(address account) external {
212:         (EpochContext memory context, ) = _settle(account);
213:
214:         UFixed18 unclaimedAmount = _unclaimed[account];
215:         UFixed18 unclaimedTotal = _totalUnclaimed;
216:         _unclaimed[account] = UFixed18Lib.ZERO;
217:         _totalUnclaimed = unclaimedTotal.sub(unclaimedAmount);
218:         emit Claim(msg.sender, account, unclaimedAmount);
219:
220:         // pro-rate if vault has less collateral than unclaimed
221:         UFixed18 claimAmount = unclaimedAmount;
222:         UFixed18 totalCollateral = _assets();
223:         if (totalCollateral.lt(unclaimedTotal)) claimAmount = claimAmount.muldiv(totalCollateral, unclaimedTotal);
224:
225:         _rebalance(context, claimAmount);
226:
227:         asset.push(account, claimAmount);
228:     }

Impact

Users are unable to obtain the remaining unclaimed collateral

Code Snippet

https://github.com/sherlock-audit/2023-05-perennial/blob/main/perennial-mono/packages/perennial-vaults/contracts/balanced/BalancedVault.sol#L211-L228

Tool used

Manual Review

Recommendation

Modify claim to allow users to claim their assets in partial amounts, or in full but ensure that the remaining unclaimed assets are still accessible for them to claim