Handle

gpersoon

Vulnerability details

Impact

The normal flow for swap transactions is: 1) user gives swap contract allowance for "inputtoken" 2) user calls swap contract (which performs the swap)

Suppose you are able to front run transaction #2 or the user has gives a large (unlimited) allowance, then you can position a transaction in between #1 and #2.

Then you can call swapByQuote() with a hacker-contract as parameter and sweep the tokens for which the user has given an allowance.

See proof of concept:

Proof of Concept

Create a hacker_token or use any other cheap token

Create a hacker_contract with following content:

function hackerfunction(amount) {
    IERC20(inputtoken).safeTransferFrom(msg.sender, address(this), amount); // retrieve input tokens from msg.sender, which is the swap contract
    IERC20(hacker_token).safeTransfer(msg.sender, 1); // give back some hacker_tokens, so swap contract is happy
    return true;
}

Call function swapByQuote() in the following way: swapByQuote(inputtoken,amount,hacker-token,1,1,hacker_contract,hacker_contract,hackerfunction&amount,block.timestamp+100) Where:

inputtoken is the tokens for which the user has given allowance to the swap contract
amount is the amount of tokens for which the allowance is given (or the max number of inputtokens that the user has if the allowance is larger)

swapByQuote() gives an allowance to hacker_contract swapByQuote() calls fillZrxQuote(), which calls hacker_contract The hacker_contract now has an allowance and can transfer the tokens from the swap contract. It should give back a token to make sure the logic in the swap contract continues.

https://github.com/code-423n4/2021-10-tally/blob/c585c214edb58486e0564cb53d87e4831959c08b/contracts/swap/Swap.sol#L106-L186

https://github.com/code-423n4/2021-10-tally/blob/c585c214edb58486e0564cb53d87e4831959c08b/contracts/swap/Swap.sol#L212

Tools Used

Recommended Mitigation Steps

Make sure the allowance by the user to the swap contract is as low as possible and/or is set to 0 after the call to swapByQuote() Use a frontrun resistant RPC to submit transactions Have a white list for allowed zrxTo addresses

code-423n4 / 2021-10-tally-findings