code-423n4 / 2022-09-frax-findings

2 stars 1 forks source link

[NAZ-H1] `syncRewards()` Can be Front-Run With A Flashloan To Force `lastRewardAmount` To Equal Zero #310

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/corddry/ERC4626/blob/643cd044fac34bcbf64e1c3790a5126fec0dbec1/src/xERC4626.sol#L78

Vulnerability details

Impact

A malicious user can front run syncRewards() with a flashloan attack to cause lastRewardAmount to equal zero. With this users will lose on rewards until the next rewardsCycleEnd.

Proof of Concept

  1. Alice calls deposit() with 1_000 tokens

    • storedTotalAssets = 1_000
    • lastRewardAmount = 100
    • asset.balanceOf(address(this)) = 1_100
  2. Mallory flashloans 800 tokens and calls deposit()

    • storedTotalAssets = 1_800
    • lastRewardAmount = 100
    • asset.balanceOf(address(this)) = 1_900
  3. Mallory then calls syncRewards()

    • storedTotalAssets = 2_000
    • lastRewardAmount = 0
    • asset.balanceOf(address(this)) = 1_900
  4. Mallory withdraws and pays back flashloan

    • Alice, with others, will now get no rewards next sync losing out on funds

Tools Used

Manual Review

Recommended Mitigation Steps

Consider adding a check for nextRewards #L85:

if (nextRewards = 0) revert ZeroValue();
FortisFortuna commented 2 years ago

From @denett A flash loan deposit will not earn you any rewards, because you are in the pool for zero seconds.

0xean commented 2 years ago

closing as invalid.