code-423n4 / 2021-04-marginswap-findings

1 stars 0 forks source link

Re-entrancy bug in `MarginRouter.crossSwapExactTokensForTokens` allows inflating balance #18

Closed code423n4 closed 3 years ago

code423n4 commented 3 years ago

Email address

mail@cmichel.io

Handle

@cmichelio

Eth address

0x6823636c2462cfdcD8d33fE53fBCD0EdbE2752ad

Vulnerability details

One can call the MarginRouter.crossSwapExactTokensForTokens function first with a fake contract disguised as a token pair: crossSwapExactTokensForTokens(0.0001 WETH, 0, [ATTACKER_CONTRACT], [WETH, WBTC]). When the amounts are computed by the amounts = UniswapStyleLib.getAmountsOut(amountIn - fees, pairs, tokens); call, the attacker contract returns fake reserves that yield 1 WBTC for the tiny input. The resulting amount is credited through registerTrade. Afterwards, _swapExactT4T([0.0001 WETH, 1 WBTC], 0, [ATTACKER_CONTRACT], [WETH, WBTC]) is called with the fake pair and token amounts. At some point _swap is called, the starting balance is stored in startingBalance, and the attacker contract call allows a re-entrancy:

pair.swap(0.0001 WETH, 1 WBTC, FUND, new bytes(0)); // can re-enter here

From the ATTACKER_CONTRACT we re-enter the MarginRouter.crossSwapExactTokensForTokens(30 WETH, 0, WETH_WBTC_PAIR, [WETH, WBTC]) function with the actual WETH <> WBTC pair contract. All checks pass, the FUND receives the actual amount, the outer _swap continues execution after the re-entrancy and the endingBalance >= startingBalance + amounts[amounts.length - 1] check passes as well because the inner swap successfully deposited these funds. We end up doing 1 real trade but being credited twice the output amount.

Impact

This allows someone to be credited multiples of the actual swap result. This can be repeated many times and finally, all tokens can be stolen.

sockdrawermoney commented 3 years ago

duplicate of #19