Open sherlock-admin opened 1 year ago
While the general claim pro-rata mechanism has already been reported in the Veriside audit (issue 002) and acknowledged by the protocol, this submission correctly adds that a user can pro-rata claim other user's assets, even if they preferred to avoid realizing losses and stay in the vault until collateral recovers. This will make users lose funds, and increase the malicious user's own amount of rewards, for when the collateral recovers.
This is a great find. We will think about whether or not we want to fix this as the unpermissioned claim is currently used by the protocol for better UX with the MultiInvoker
Fixed: https://github.com/equilibria-xyz/perennial-mono/pull/206 - this change only allows "self" claims if the vault is pro-rated
tvdung94
medium
Users can be forced to claim assets at bad rate in some cases
Summary
In balanced vault contract, anyone can call claim assets for other accounts; malicious users could abuse this function to force other users to receive less assets than they expect.
Vulnerability Detail
When claiming asset, pro rate, in which users will receive less assets than expect, will be applied when total collateral is less than total unclaimed amount. So after redeeming shares and converting it to assets, users might not want to claim assets right away in this scenario, for they will receive less token amount. However, other users can force them to claim via claim() function because there is no restrict on this function to claim for other accounts.
Impact
Users will be forced to receive less assets in some scenarios.
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
Only account owner (msg.sender == account) can claim asset for their account.