Zaros Part 1

Summary

The withdrawMarginUsd function in the TradingAccount library is vulnerable to reentrancy attacks. An attacker can exploit this vulnerability to repeatedly withdraw funds before the contract state is updated, potentially draining the contract of its assets.

Vulnerability Details

https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/leaves/TradingAccount.sol#L434-L479

External Call: IERC20.safeTransfer(recipient, amountToTransfer);

1. Setup:

• An attacker deposits a certain amount of collateral into their trading account.

• The attacker then calls the `withdrawMarginUsd` function to withdraw this collateral.

2. Attack Contract:

• The attacker deploys a malicious contract designed to exploit the reentrancy vulnerability.

• This malicious contract has a fallback function that re-calls the `withdrawMarginUsd` function when it receives tokens.

3. Execution:

• The attacker initiates a withdrawal by calling `withdrawMarginUsd` from their malicious contract.

• The `withdrawMarginUsd` function starts executing and reaches the point where it transfers tokens using `IERC20.safeTransfer`.

4. Reentrancy:

• During the `safeTransfer` call, the attacker's contract receives tokens and its fallback function is triggered.

• The fallback function immediately calls `withdrawMarginUsd` again before the first call completes and updates the state.

• The second call to `withdrawMarginUsd` proceeds as if the first call had not happened, allowing the attacker to withdraw more tokens.

5. Repeat:

• This process can repeat multiple times, draining the contract of tokens before the state is correctly updated to reflect the withdrawals.

Impact

• Financial Loss

• Trust Issues

• Operational Disruption

Tools Used

Manual review

Recommendations

Implement the nonReentrant modifier from OpenZeppelin's ReentrancyGuard to prevent reentrant calls.