The addHolding function does not update the account.holdsToken map.
This leads to a critical vulnerability where deposits of the same token keep being pushed to the account.holdingTokens array but the sum is correctly updated in account.holdings[token]. However, because of the duplicate token in the holdingTokens array the same token is counted several times in the getHoldingAmounts function:
The MarginRouter.crossCloseAccount function uses these wrong amounts to withdraw all tokens:
An attacker can just deposit the same token X times which increases their balance by X times the actual value.
This inflated balance can then be withdrawn to steal all tokens.
Recommend correctly setting the account.holdsToken map in addHolding.
