PoolTogether Aave v3 contest - berndartmueller's results

Lines of code

https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L361

Vulnerability details

Impact

The first depositor into the yield source is able to maliciously manipulate the share price by depositing the lowest possible amount (1 wei) and then artificially blowing up the token balance. Following depositors will have to supply a larger amount of tokens to receive shares > 0.

No funds are lost as deposits are checked if received shares > 0 (see AaveV3YieldSource.sol#L233) but it prevents supplying smaller amounts of assets.

This is a well known issue, Uniswap and other protocols had similar issues when supply == 0.

Proof of Concept

1. Alice (attacker) deposits 1 wei via supplyTokenTo()
2. Alice receives 1 wei shares
3. Alice transfers (donates) 1000 aToken via transfer() to yield source contract to artificially blow up balance without minting new shares. Balance is now 1001 aToken -> share price is now very high (= 1001 wei)
4. Bob has to supply larger amount of assets to receive shares

The formula _supply == 0 ? _tokens : _tokens.mul(_supply).div(balance) (see AaveV3YieldSource.sol#L361) calculates for deposits of e.g. 100 asset tokens in case of such a high share price the following:

(100 * 1) / 1001 => shares = 0 (precision issues)

This is due to balance > (_tokens * _supply).

Tools Used

Manual review

Recommended mitigation steps

For the first, initial, deposit, mint fixed amount of shares, e.g. 10**decimals() instead of minting same amount of supplied _tokens.

Similarly, for _sharesToToken(), return fixed amount of tokens, e.g. 10**decimals() if _supply == 0

Fix inspired by Uniswap V2 and Harvest Finance

Lines of code

https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L237

Vulnerability details

Impact

There are ERC20 tokens that may make certain customizations to their ERC20 contracts. One type of these tokens is deflationary tokens that charge a certain fee for every transfer() or transferFrom().

Proof of Concept

AaveV3YieldSource.sol#L237

Tools Used

Manual review

Recommended mitigation steps

Get the actual received amount by calculating the difference of token balance before and after the transfer. e.g.:

uint256 _beforeBalance = _underlyingAssetAddress.balanceOf(address(this));
IERC20(_underlyingAssetAddress).safeTransferFrom(msg.sender, address(this), _depositAmount);
uint256 _afterBalance = _underlyingAssetAddress.balanceOf(address(this));