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

0 stars 0 forks source link

QA Report #56

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Summary

We list 1 low-critical finding and 1 non-critical finding:

(Low) _requireNotAToken should check the token address != address(0)

Impact

The decreaseERC20Allowance, increaseERC20Allowance and transferERC20 functions should check the token address != address(0).

Proof of Concept

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

Tools Used

vim

Recommended Mitigation Steps

Check address(_token) != address(0).

(Non) Duplicate code of address check

Impact

In transferERC20, it checks that the _token address should not be aToken, which is implemented in _requireNotAToken.

Proof of Concept

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

Tools Used

vim

Recommended Mitigation Steps

Use _requireNotAToken in the transferERC20 function.

PierrickGT commented 2 years ago

(Low) _requireNotAToken should check the token address != address(0)

These functions are only callable by the owner or manager, so it's ok if we don't check for address zero.

(Non) Duplicate code of address check

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