code-423n4 / 2023-09-asymmetry-findings

2 stars 1 forks source link

VotiumStrategyCore.applyRewards can be sandwhiched to make profit #9

Closed c4-submissions closed 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-09-asymmetry/blob/main/contracts/strategies/votium/VotiumStrategyCore.sol#L272-L305

Vulnerability details

Impact

VotiumStrategyCore.applyRewards can be sandwhiched to make profit.

Proof of Concept

VotiumStrategyCore.applyRewards function will swap all rewards of contract into eth and then stake them into safEth or vEth contract. As result price of afEth token will increase.

Usually, when user want to withdraw, then he needs to make withdraw request, which will calculate when it will be possible to withdraw. So on that timestamp user will then call withdraw. So this is usually 2 step process that can't be done in same tx.

But it's also possible that on the moment of VotiumStrategyCore.applyRewards function call, unlockable amount will have some gap, that user can use to withdraw instantly.

In this case user can just deposit that gap amount right before, VotiumStrategyCore.applyRewards call and then request withdraw and withdraw, all in same block. This will be easy profit for use, but condition should satisfy: unlockable should have enough gap, so user can earn at least more than tx cost on this attack.

Tools Used

VsCode

Recommended Mitigation Steps

I don't know good solution.

Assessed type

Error

elmutt commented 1 year ago

super interesting find. Will discuss with the team the implications of this

elmutt commented 1 year ago

@rvierdiyev I think I understand the exploit but want to be sure. Here is an example how I see it:

1) unlockable = 999 cvx 2) exploiter sees applyRewards() being called in the mempool. 3) exploiter front runs and calls deposit() first in such a way that withdraw() is expected to withdraw 999 cvx after applyReward() has been called. 4) exploiter calls withdraw() in same block and profits

Am I understanding the problem correctly?

rvierdiiev commented 1 year ago

@elmutt yes, but just unlockable = 999 cvx is not enough, as we have such thing as cvxUnlockObligations as well(amount that is already reserved). let's say cvxUnlockObligations is 499, so by gap i mean unlockable - cvxUnlockObligations (as new withdrawer can't pretend on obligations to other people), which is 500 in our case everything else is described correctly

c4-judge commented 1 year ago

0xleastwood marked the issue as primary issue

c4-judge commented 1 year ago

0xleastwood marked the issue as duplicate of #45

c4-judge commented 1 year ago

0xleastwood marked the issue as satisfactory