`depositMargin` is vulnerable to a re-org attack causing trader to lose their margin deposit
Summary
A re-org event can cause a trader to deposit margin to the wrong account, permanently losing their funds
Vulnerability Details
Consider the following unfinalized blocks:
Block 1: Alice calls createTradingAccount and is assigned tradingAccountId = 1
Block 2: Bob calls createTradingAccount and is assigned tradingAccountId = 2
Block 3: Alice calls depositMargin passing in tradingAccountId = 1
A re-org event occurs, placing block 2 before block 1. The new order of execution is:
Block 2 is executed. Bob calls createTradingAccount and is assigned tradingAccountId = 1
Block 1 is executed. Alice calls createTradingAccount and is assigned tradingAccountId = 2
Block 3 is executed. Alice calls depositMargin passing in tradingAccountId = 1, BUT now she has deposited margin into Bob's account since he owns tradingAccountId = 1
Impact
Trader loses their funds by depositing to another account
Likelihood = Low, since re-orgs are rare
Impact = High, significant loss of funds
Tools Used
Manual review
Recommendations
Implement a depositToMe function so that it check the trading account associated with msg.sender and deposits margin there
Lead Judging Commences
Traders calling createTradingAccount + depositMargin can lose their margins in case of a chain reorg.
Appeal created
Traders calling createTradingAccount + depositMargin can lose their margins in case of a chain reorg.
Prize pool breakdown
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.