search

code-423n4 / 2023-05-venus-findings

2 stars 1 forks source link

Missing transaction expiration check result in asset tokens selling at a lower price #522

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-venus/blob/main/contracts/RiskFund/RiskFund.sol#L265

Vulnerability details

Impact

Selling of asset tokens misses the transaction expiration check, which may lead to reward tokens being sold at a price that's lower than the market price at the moment of a swap.

Proof of Concept

The _swapAsset() function, which is responsible for selling tokens on Pancackeswap, sets the deadline argument of the amountOutMin call to block.timestamp, which basically disables the transaction expiration check because the deadline will be set to whatever timestamp the block including the transaction is minted at.

Transaction expiration check (implemented in Pancackeswap via the deadline argument) allows users of Pancackeswap to protect from selling tokens at an outdated price that's lower than the current price. Consider this scenario:

  1. The RiskFund.swapPoolsAssets() function is called.
  2. Before the transaction is mined, there's a rapid increase of gas cost. The transaction remains in the mempool for some time since the gas cost paid by the transaction is lower than the current gas price.
  3. While the transaction is in the mempool, the price of the baseToken increases.
  4. After a while, gas cost drops and the transaction is mined. Since the value of amountOutMin was calculated based on an outdated baseToken price which is now lower than the current price, the swapping is sandwiched by a MEV bot. The bot decreases the price of the reward token in a Pancackeswap pool so than the minimum output amount check still holds and earns a profit from the swapping happing at a lower price.
  5. As a result of the sandwich attack, asset tokens are swapped at an outdated price, which is now lower than the current price of the baseTokens. The protocol earn less than they could've received by selling the tokens at the current price.

Tools Used

Manual Audit

Recommended Mitigation Steps

Consider a reasonable value to the deadline argument. For example, Uniswap sets it to 30 minutes on the Ethereum mainnet and to 5 minutes on L2 networks, taking as reference those values, estimate how much time would be a reasonable value for the BSC chain. Also consider letting the caller to change the value when on-chain conditions change and may require a different value.

Assessed type

MEV

c4-judge commented 1 year ago

0xean marked the issue as duplicate of #167

c4-judge commented 1 year ago

0xean marked the issue as satisfactory