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

Manual deposits can manipulate share price #105

Closed
code423n4 opened this issue Jun 24, 2021 · 2 comments
Closed

Manual deposits can manipulate share price #105

code423n4 opened this issue Jun 24, 2021 · 2 comments
Labels

Comments

@code423n4
Copy link
Contributor

Handle

tensors

Vulnerability details

Impact

Increasing/decreasing the balance of tokens in the pool by manually depositing them changes the values of the shares.

Proof of Concept

https://github.com/pooltogether/aave-yield-source/blob/bc65c875f62235b7af55ede92231a495ba091a47/contracts/yield-source/ATokenYieldSource.sol#L147-L149

https://github.com/pooltogether/aave-yield-source/blob/bc65c875f62235b7af55ede92231a495ba091a47/contracts/yield-source/ATokenYieldSource.sol#L164-L166

Suppose that before I swap my shares (S in total ) for tokens (T in total) I deposit X tokens to the pool without getting shares for them.

By the shares to tokens formula, if S(A+X)/T -X > 0 I can take a profit from artificially increasing the price.

If I have some mechanism to withdraw the tokens X, that I deposited then it is always profitable to manipulate the price of the shares. I couldn't find such a mechanism in the code, but maybe someone else did.

Recommended Mitigation Steps

Record the price gained through interest alone, or don't allow deposits from unknown sources.

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

By depositing tokens directly into the yield source, you're effectively distributing those tokens over all shareholders. There is nothing to be gained by depositing.

@dmvt
Copy link
Collaborator

dmvt commented Aug 27, 2021

Vulnerability relies on a non-existent withdraw mechanism. Attempting this exploit would benefit, not harm, all users. Closing.

@dmvt dmvt closed this as completed Aug 27, 2021
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