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

2 stars 1 forks source link

Malicious users can steal the rewards from the vault #267

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L177-L178 https://github.com/code-423n4/2022-09-frax/blob/main/src/frxETHMinter.sol#L87 https://github.com/corddry/ERC4626/blob/643cd044fac34bcbf64e1c3790a5126fec0dbec1/src/xERC4626.sol#L78-L97

Vulnerability details

Impact

malicious users will steal the rewards from the vault sfrxETH.sol

Proof of Concept

On frxETHMinter.sol: submitPaused == true for the last three weeks. so no one can _submit() eth even to convert rewards by Multisig Treasury Contract
All these rewards for the past 3 weeks are staked on the Multisig Treasury Contract

On sfrxETH.sol: syncRewards() was invoked three times (as we know Multisig Treasury Contract have no frxETH to transfer it).

On frxETHMinter.sol: At the end of the fourth week, submitPaused will be submitPaused == false So now Multisig Treasury Contract can convert ETH to frxETH At the same time, Alice will submit() an amount of ETH

On sfrxETH.sol: When (block.timestamp > rewardsCycleEnd) == true And before invoking syncRewards() by anyone or the bot (or the bot is not working) Alice will deposit() or mint() all the frxETH to sfrxETH Now after one-day syncRewards() is invoked successfully Alice now can redeem() or withdraw() with a higher exchange rate and steal the reward of the last month even though he only was deposited yesterday in the vault

Recommended Mitigation Steps

In case the submitPaused == true for long time The Multisig Treasury Contract need to be only able to _submit() And then someone needs to invoke syncRewards() After that opens, the _submit() for the public users

FortisFortuna commented 2 years ago

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 commented 2 years ago

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