Yield source unnecesarily loses money to fees

[code423n4]

Lines of code

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

Vulnerability details

Impact

The current implementation withdraws the underlying asset to the yield source contract before transferring it to the msg.sender. This extra step unnecessarily incurs a fee which will likely cause problems for the recipient contract.

Proof of Concept

File: contracts/AaveV3YieldSource.sol

258       uint256 _beforeBalance = _assetToken.balanceOf(address(this));
259       _pool().withdraw(_underlyingAssetAddress, _redeemAmount, address(this));
260       uint256 _afterBalance = _assetToken.balanceOf(address(this));
261   
262       uint256 _balanceDiff = _afterBalance.sub(_beforeBalance);
263       _assetToken.safeTransfer(msg.sender, _balanceDiff);

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

Tools Used

Code inspection

Recommended Mitigation Steps

Rather than using address(this) as the withdraw() recipient, use the final destination, msg.sender instead

[PierrickGT]

We withdraw to the yield source first to be able to calculate the actual amount of tokens that was transferred by the Aave pool and not only assume that the amount returned by the withdraw function is the correct amount. So for this reason, I have disputed the issue.

[gititGoro]

Fees are naturally incorporated in calculations so this looks more like a dev decision than a risk. Marking as invalid.