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

Reentrancy on distributeOther() #9

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

Reentrancy on distributeOther() #9

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

Comments

@code423n4
Copy link
Contributor

Handle

tensors

Vulnerability details

Impact

The distribute function can be re-entered by fake tokens or tokens with callbacks.
An attacker can use the callbacks on safeTransfer if a token has a callback to reenter an drain the entire balance of
that particular token before the notifyRewardAmount is called.

I think this is only a medium issue because the attacker can only take tokens with callbacks, and I don't think any of the tokens you guys use have callbacks.

Proof of Concept

https://github.com/code-423n4/2021-09-bvecvx/blob/1d64bd58c7a4224cc330cef283561e90ae6a3cf5/veCVX/contracts/locker/CvxStakingProxy.sol#L153

Recommended Mitigation Steps

Be aware of this possibility. If you really want to, add a nonReentrant modifier to the function.

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

Not my issue to handle, but should be low because it's dependent on the token, also you already know cvxCRV and CVX are ERC20 and don't have custom hooks on transfer

@ghoul-sol
Copy link
Collaborator

as per warden description this is unlikely to happen so making this low risk

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

4 participants