To our knowledge, this can be exploited as of now for only the smallest unit of the token underlying, so for example one millionth of an USDC at a time. As the aTokens, the shares, and the token underlying shares the same amount of decimals the calculation is actually (amount of tokens to withdraw (X 10 * 6) / ((X + some interest) 10 ** 6). Due to the nature of this calculation, "amount of tokens to withdraw" should be 1 for this calculation to return 0 in our understanding of the issue. While this is probably not much of an issue, we are reporting this as exploiting it might be meaningful in future and it has an easy fix.
PierrickGT
commented
2 years ago
Lines of code
https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L255
Vulnerability details
Impact
Calculation without the bigger than zero check may result in loss of user funds, albeit in tiny amounts as of now.
Proof of Concept
In this line of redeemToken (https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L256) shares to burn is calculated through _tokenToShares method (https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L357). As there is no check that checks if amount of _shares is larger than 0 like the one in supplyTokenTo (https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L233), _tokenToShares (https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L357) may return 0 and this may result in withdrawal of funds without burning any shares.
To our knowledge, this can be exploited as of now for only the smallest unit of the token underlying, so for example one millionth of an USDC at a time. As the aTokens, the shares, and the token underlying shares the same amount of decimals the calculation is actually (amount of tokens to withdraw (X 10 * 6) / ((X + some interest) 10 ** 6). Due to the nature of this calculation, "amount of tokens to withdraw" should be 1 for this calculation to return 0 in our understanding of the issue. While this is probably not much of an issue, we are reporting this as exploiting it might be meaningful in future and it has an easy fix.
Tools Used
Manual code review, talks with dev
Recommended Mitigation Steps
Implement a require check like the one in supplyTokenTo (https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L233)