-
Notifications
You must be signed in to change notification settings - Fork 0
Open
Description
Sneaky Ginger Falcon
High
Incorrect Pending Shares Calculation
Location: modifyPendingAssets() function
Description:
/// @inheritdoc IRiskManager
function modifyPendingAssets(address asset, int256 change) external onlyQueueOrRole(MODIFY_PENDING_ASSETS_ROLE) {
RiskManagerStorage storage $ = _riskManagerStorage();
int256 pendingAssetsBefore = $.pendingAssets[asset];
int256 pendingSharesBefore = $.pendingShares[asset];
int256 pendingAssetsAfter = pendingAssetsBefore + change;
int256 pendingSharesAfter = convertToShares(asset, pendingAssetsAfter);
int256 shares = pendingSharesAfter - pendingSharesBefore;
int256 newPendingBalance = $.pendingBalance + shares;
if (shares > 0 && $.vaultState.balance + newPendingBalance > $.vaultState.limit) {
revert LimitExceeded($.vaultState.balance + newPendingBalance, $.vaultState.limit);
}
$.pendingAssets[asset] = pendingAssetsAfter;
$.pendingShares[asset] = pendingSharesAfter;
$.pendingBalance = newPendingBalance;
emit ModifyPendingAssets(asset, change, pendingAssetsAfter, pendingSharesAfter);
}
-
Global pendingBalance is the sum of all pendingShares[asset]
-
pendingShares[asset] are recalculated during every modification
-
Price changes between operations cause imbalance between global and per-asset tracking
-
Entire pending shares are recalculated from total assets instead of delta
-
Price changes cause incorrect share calculations and limit enforcement
Impact:
Incorrect risk limit enforcement
Potential fund loss through improper deposit/withdrawal processing
Step-by-Step Exploitation:
- User deposits 100 USDC when 1 USDC = 1 share
pendingShares[USDC] = 100pendingBalance = 100
- Oracle price changes: 1 USDC = 1.1 shares
- User cancels deposit via
modifyPendingAssets(-100 USDC)- Recalculation:
pendingSharesAfter = -110 shares pendingBalance = 100 + (-110) = -10
- Recalculation:
- Result:
- Global pending balance is -10 shares
- No corresponding asset liability
- Subsequent deposits locked in contract
Fix: Calculate shares based on delta, not total:
int256 shares = convertToShares(asset, change);
int256 pendingSharesAfter = pendingSharesBefore + shares;Metadata
Metadata
Assignees
Labels
No labels