Owner of the `PoolAddressesProviderRegistry` Contract Can Update the Pool Address and Effectively Lock Deposited Funds by Preventing All Withdrawals

[code423n4]

Lines of code

https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L259 https://github.com/pooltogether/aave-v3-yield-source/blob/e63d1b0e396a5bce89f093630c282ca1c6627e44/contracts/AaveV3YieldSource.sol#L388-L401 https://github.com/aave/aave-v3-core/blob/master/contracts/protocol/configuration/PoolAddressesProviderRegistry.sol

Vulnerability details

Impact

The owner of the PoolAddressesProviderRegistry contract is able to register and unregister providers as they see fit. Because AaveV3YieldSource.sol dynamically queries the Aave pool through this contract, it is possible for the owner of this Aave contract to effectively rug all deposited funds by unregistering the pool, causing redeemToken() to revert indefinitely. This is effectively a DoS attack, resulting in a users' funds being completely locked.

Tools Used

Manual code review.

Recommended Mitigation Steps

Consider querying the Aave pool from the PoolAddressesProviderRegistry contract and loading this into storage only once. This should prevent all future updates to this address, potentially causing a loss in users' funds. It's also important to note that _pool() can be manipulated to steal funds deposited through the supplyTokenTo() function.

[PierrickGT]

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

If the pool address changes, I expect the funds held by the pool to be migrated to a new pool. We can then use the increaseERC20Allowance to approve this new contract.

[gititGoro]

Confirming duplicate and therefore marking invalid.