Platform: Code4rena
Start Date: 29/04/2022
Pot Size: $22,000 USDC
Total HM: 6
Participants: 40
Period: 3 days
Judge: Justin Goro
Total Solo HM: 2
Id: 114
League: ETH
Rank: 16/40
Findings: 2
Award: $255.21
馃専 Selected for report: 0
馃殌 Solo Findings: 0
226.8573 USDC - $226.86
If native assets such as ETH or MATIC are for some reason deposited in the contract (via selfdestruct) they will be locked/lost as there is no way to withdraw them from the contract.
I suggest implementing a withdraw function to manage native assets that could have been mistakenly sent to the contract.
AaveV3YieldSourceIf attacker directly sends aTokens to AaveV3YieldSource it will change the 1:1 ratio in _tokenToShares. Consequently users will not get 1:1 shares when depositing ERC20s. This might confuse users into thinking they wont get their fair share back when calling redeemToken, which could be true if they deposited before the attacker directly sent aTokens to AaveV3YieldSource, but not after.
I recommend implementing checks to ensure aTokens held in contract have a 1:1 ratio to total shares before redeeming asset tokens. In addition implement a function that can rebalance amount of aToken held by contract if check above is false.
Function state mutability can be restricted to view
function balanceOfToken(address _user) external override returns (uint256)
Function does not makes any state changes and therefore can be restricted to view.
#0 - PierrickGT
2022-05-03T15:27:18Z
L-01: No way to withdraw native assets
It would be an unfortunate mistake but this contract should never old native assets, that's why we won't implement a function to withdraw native assets.
L-02: 1:1 Ratio can be manipulated by artifically inflating aToken balance of AaveV3YieldSource
This scenario is not plausible, as explained in the following issue: https://github.com/code-423n4/2022-04-pooltogether-findings/issues/23
N-01: Function state mutability can be restricted to view
As mentioned in another issue, we inherit from the yield source interface, so we sadly can't restrict this function to view.
For the reasons above, I have acknowledged the issue but we won't implement the suggestions of the warden.
#1 - gititGoro
2022-05-17T00:10:13Z
Note: Dev has restricted visibility of inherited balanceOfToken with PR and the issue. https://github.com/pooltogether/aave-v3-yield-source/pull/16/commits/e583ca0bf31b35eaab9c4dd12872d94978afb408
馃専 Selected for report: IllIllI
Also found by: 0v3rf10w, 0x1f8b, 0x4non, 0xDjango, 0xf15ers, 0xkatana, 242, Dravee, GimelSec, MaratCerby, Tadashi, TrungOre, WatchPug, defsec, fatherOfBlocks, gzeon, hake, horsefacts, joestakey, miguelmtzinf, pauliax, pedroais, peritoflores, rotcivegaf, simon135, slywaters, tabish, throttle, z3s
28.3489 USDC - $28.35
GAS
Using safeMath: Contract 路 Method 路 Min 路 Max 路 Avg 路 # cal | AaveV3YieldSourceHarness 路 redeemToken 路 - 路 - 路 116178 路 | AaveV3YieldSourceHarness 路 supplyTokenTo 路 115041 路 157449 路 146161 路
Not using SafeMath: | AaveV3YieldSourceHarness 路 redeemToken 路 - 路 - 路 115989 路 | AaveV3YieldSourceHarness 路 supplyTokenTo 路 114915 路 157449 路 146119 路
Difference: redeemToken = 116178 - 115989 = 189
supplyTokenTo (avg) = 146161 - 146119 = 42
#0 - PierrickGT
2022-05-03T15:28:21Z
Great report. This user should receive bonus points for it.
#1 - PierrickGT
2022-05-03T15:29:01Z