Lucky Peach Barbel
Medium
The _update function handles deposits, redemptions, and claims for a given account. It ensures that the operations are single-sided (i.e. only one of deposit, redemption, or claim can be performed at a time) and enforces various invariants. However, there is a critical flaw in the logic that checks the invariants, specifically in the validation of depositAssets and redeemShares.
The invariant check in the _update function is as follows:
if (!depositAssets.add(redeemShares).add(claimAssets).eq(depositAssets.max(redeemShares).max(claimAssets)))
revert VaultNotSingleSidedError();The intention of this check is to ensure that only one of depositAssets, redeemShares, or claimAssets is non-zero at a time, enforcing single-sided operations. However, the logic is flawed because it does not correctly handle the case where two or more of these values are non-zero but their sum equals the maximum value among them.
An attacker could potentially perform multiple operations (e.g., deposit and redeem) simultaneously by carefully choosing values for depositAssets, redeemShares, and claimAssets that satisfy the flawed invariant check. This could lead to unexpected behavior in the vault's accounting, such as incorrect updates to the user's shares or assets, potentially resulting in financial losses or other issues.
Example Scenario:
An attacker wants to both deposit and redeem assets in the same transaction. The attacker sets depositAssets = 100, redeemShares = 100, and claimAssets = 100. The invariant check 100 + 100 + 100 == 100.max(100).max(100) evaluates to 300 == 100, which should fail. However, due to the flawed logic, the check might incorrectly pass, allowing the attacker to perform both deposit and redemption in the same transaction.
the invariant check should be modified to ensure that only one of depositAssets, redeemShares, or claimAssets is non-zero. This can be done by checking that the sum of the non-zero counts is exactly one.