code-423n4 / 2022-04-pooltogether-findings

0 stars 0 forks source link

Missing support for ERC20 with fee #64

Closed code423n4 closed 2 years ago

code423n4 commented 2 years ago

Lines of code

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

Vulnerability details

Impact

Contract AaveV3YieldSource allows depositing tokens via supplyTokenTo function. Amount of tokens to transfer is based on passed argument _depositAmount and is missing support for tokens with built-in fees. One of the popular tokens that implements such a fee (but currently is set to 0) is USDT.

The severity of vulnerability has been raised to medium since the "areas of concerns" includes shares/tokens peg at 1:1 ratio.

Proof of Concept

Used Tools

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to add support for ERC20 tokens with built-in fees. Example of the implementation:

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

uint256 receivedTokens = _afterBalance - _beforeBalance
(..)
PierrickGT commented 2 years ago

Duplicate of https://github.com/code-423n4/2022-04-pooltogether-findings/issues/8