flash loan attack risk increases as the time span increases between lastSync and nextSync before synRewards is called

[code423n4]

Lines of code

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

Vulnerability details

Impact

flash loan attack risk increases as the time span increases between lastSync and nextSync before synRewards is called

Proof of Concept

as time passes since last sync, a flash loan attack becomes increasingly profittable. One can take a floan loan of asset, deposit into sfrxETH, at which point, shares conversion is based on (storedTotalAssets + lastRewardAmount). At the end of deposit, syncRewards will be called. Now withdraw conversion will be based on (storedTotalAssets + unlockedRewards), where unlockedRewards is based on (block.timestamp - lastSync). With a large flash loan, the attacker can basically retrieve most of rewards during (block.timestamp - lastSync_) period, hurting long term depositers https://github.com/corddry/ERC4626/blob/643cd044fac34bcbf64e1c3790a5126fec0dbec1/src/xERC4626.sol#L45

Tools Used

Recommended Mitigation Steps

call syncRewards() before totalAssets() in withdraw, deposit functions.

[FortisFortuna]

From @denett syncRewards should be called by us at the beginning of each period, or we need to automatically call it before deposits/withdrawals.

[FortisFortuna]

Duplicate of https://github.com/code-423n4/2022-09-frax-findings/issues/110

[141345]

But flahloan has to be paid back within the same tx, then the vault shares is returned back. The point is, only self owned fund can be used in this vector.

[0xean]

closing as invalid, the flashloan attack doesn't work here