Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ManualRebalance will be frontrun for most of the tokens. #5

Open
code423n4 opened this issue Sep 6, 2021 · 3 comments
Open

ManualRebalance will be frontrun for most of the tokens. #5

code423n4 opened this issue Sep 6, 2021 · 3 comments
Labels

Comments

@code423n4
Copy link
Contributor

Handle

tensors

Vulnerability details

Impact

We have previously seen that the harvest function can be exploited for almost all the tokens at stake.
Since ManualRebalance calls harvest, it is also unsafe and funds swapped using it will likely be lost.

Proof of Concept

https://github.com/code-423n4/2021-09-bvecvx/blob/1d64bd58c7a4224cc330cef283561e90ae6a3cf5/veCVX/contracts/veCVXStrategy.sol#L444-L453

Recommended Mitigation Steps

Adding an amount out minimum here will work that should be passed on to the harvest method.

@code423n4 code423n4 added 3 (High Risk) bug Something isn't working labels Sep 6, 2021
code423n4 added a commit that referenced this issue Sep 6, 2021
@GalloDaSballo
Copy link
Collaborator

Disagree with risk (should be medium like all other harvest findings), also we have optional harvest which means we can skip it, hence the finding is deceiving at best

@GalloDaSballo
Copy link
Collaborator

We use private txs to mitigate

@ghoul-sol
Copy link
Collaborator

similar to #55 #56 and other front-running issues in this contest, it's low risk given the sponsor comments.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants