# KangarooVault.removeCollateral updates storage without actually removing collateral, resulting in lost collateral

KangarooVault.removeCollateral decremented totalCollateral and usedFunds but never called EXCHANGE.removeCollateral, so the collateral stayed in the exchange and was lost on position close.

- Payout: Contest reward
- Severity: high
- Program: Polynomial Protocol
- Credited: joestakey
- Disclosed: 2023-08-01
- Write-up quality: 7/10
- Original: https://github.com/code-423n4/2023-03-polynomial-findings/issues/189
- Library page: https://bugbountylibrary.com/r/c4%3A2023-03-polynomial%3AH-04
- Tags: domain:web3 impact:funds-loss kind:contest platform:code4rena sev:high tech:evm tech:solidity technique:source-review vuln:accounting-error

**Target:** KangarooVault.sol removeCollateral / Exchange collateral accounting
**Root cause:** removeCollateral only updates usedFunds and positionData.totalCollateral in storage without invoking EXCHANGE.removeCollateral, so the exchange still holds the collateral while the vault's accounting forgets it.
**Steps:**
1. Open a position via `kangaroo.openPosition(amt, 0)` and execute/clear orders.
2. Record `susd.balanceOf(address(kangaroo))` and `positionData.totalCollateral`.
3. Call `kangaroo.addCollateral(collDelta)`; balance decreases by collDelta and totalCollateral increases.
4. Call `kangaroo.removeCollateral(collDelta)`; assert SUSD balance delta is 0 while totalCollateral drops back to initial.
5. Close the position: `_closePosition` sets `tradeParams.collateralAmount = positionData.totalCollateral`, so `ShortCollateral.sendCollateral` returns only the reduced amount, stranding the removed collateral in the exchange.
**How found:** Code review of the addCollateral/removeCollateral pair: addCollateral calls EXCHANGE.addCollateral but removeCollateral only mutates storage, prompting a Foundry test asserting the SUSD balance is unchanged after removal.
**Exploitation:** Admin-triggered accounting bug: after removeCollateral, usedFunds is understated so processWithdrawalQueue computes availableFunds above real SUSD balance and reverts; on close, collateralAmount is capped by the decremented totalCollateral, so the removed collateral is never returned to the vault and is effectively lost.
**Insight:** When a wrapper function mirrors an add/remove pair, diff the two implementations for asymmetric external calls; a storage-only decrement silently desyncs vault accounting from the exchange's real collateral, and the loss surfaces later in closeTrade's collateralAmount cap.