search

code-423n4 / 2022-01-timeswap-findings

2 stars 0 forks source link

No slippage control causes sandwich attack #178

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Handle

sirhashalot

Vulnerability details

Impact

The Timeswap protocol is a DEX and therefore should protect users against frontrunning and sandwich attacks. The approach used by Uniswap is to set bounds on the slippage that can occur in a trade by specifying a minimum amount for the DEX user to receive, as seen in the Uniswap addLiquity function. An attacker could use a flashloan to cause a large change in liquidity of a trading pair, causing a change in price for the pair. Since a user cannot set a minimum value they expect to receive from Timeswap, they would receive far fewer assets than expected, and the attacker can profit.

Here is a similar finding in a public audit and here is an example of this weakness in a DEX.

Proof of Concept

The functions that add liquidity in TimeswapConvenience.sol should have some guarantees for the user to receive a minimum amount of value, but no guarantees or minimum values are used. While there are many similar functions, one example is the liquidityGivenAsset() function in TimeswapConvenience.sol. The function calls liquidityGivenAsset() -> _liquidityGivenAsset() -> _mint() -> TimeswapPair.mint(), and the mint() function in TimeswapPair.sol makes no guarantees about the value of the liquidityOut value it returns. There is no way for a user to specify the minimum lidquidityOut value they expect to receive.

Recommended Mitigation Steps

It is unclear how a DEX without an oracle can solve this problem without limiting the speed of transactions. Using a time-weighted average price (TWAP) may help reduce the risk of very dramatic price changes due to frontrunning, but would not remove the risk. Another option may be to limit the number of Timeswap transactions in a block or period of time, but this option can result in temporarily denial of service (DoS) to valid users.

Mathepreneur commented 2 years ago

The functions in the convenience contract has slippage protection mechanism, like minLiquidity, maxDebt, and maxCollateral for the liquidityGivenAsset, which is implemented in the function _liquidityGivenAsset in the Mint.sol library. Also Timeswap is not a DEX, but a lending/borrowing protocol.

0xean commented 2 years ago 
        require(liquidityOut >= params.minLiquidity, 'E511');
        require(dueOut.debt <= params.maxDebt, 'E512');
        require(dueOut.collateral <= params.maxCollateral, 'E513');

closing as invalid