# account.holdsToken is never set

Marginswap's addHolding never sets holdsToken, so repeated deposits of one token duplicate array entries and inflate withdrawable balance in crossCloseAccount, letting attackers drain funds.

- Payout: Audit contest
- Severity: high
- Program: Marginswap
- Disclosed: 2021-05-03
- Write-up quality: 7/10
- Original: https://github.com/code-423n4/2021-04-marginswap-findings/issues/25
- Library page: https://bugbountylibrary.com/r/c4%3A2021-04-marginswap%3AH-07
- Tags: area:web3 domain:web3 impact:funds-loss kind:contest platform:code4rena sev:high tech:evm tech:solidity vuln:accounting-error vuln:business-logic

**Target:** CrossMarginAccount / MarginRouter.crossCloseAccount (Solidity smart contract)
**Root cause:** addHolding pushes a token to account.holdingTokens without setting account.holdsToken, so the hasHoldingToken guard never trips and the same token is pushed repeatedly; getHoldingAmounts then returns the full holdings[token] balance once per duplicate entry.
**Steps:**
1. Deposit the same token X times via addHolding; each call pushes token again into account.holdingTokens because holdsToken is never set.
2. Call getHoldingAmounts(trader): holdingTokens contains X duplicate entries, each mapped to the full account.holdings[token] balance.
3. Call MarginRouter.crossCloseAccount(); it loops over holdingTokens and calls Fund.withdraw(holdingTokens[i], msg.sender, holdingAmounts[i]) for each duplicate, withdrawing X times the real balance.
**How found:** Code review of addHolding noticed the missing assignment to account.holdsToken (comment marks where it should be set), then traced how duplicate holdingTokens entries propagate into getHoldingAmounts and crossCloseAccount.
**Exploitation:** Attacker deposits one token multiple times to create duplicate holdingTokens entries, then calls crossCloseAccount which withdraws holdings[token] once per duplicate entry, extracting X times the deposited value and draining other tokens held by the Fund.
**Insight:** When a guard map (holdsToken) is meant to dedupe an array, verify it is actually written; then trace every consumer of that array for per-entry balance lookups that multiply value.