ParagonsDAO - Sleeves

The `PrimeRewards` contract permits a single token ID to be registered in multiple pools, causing `tokenIdsToPoolIds` to return the most recently listed pool, overwriting earlier entries when executing `Sleeves.adminSyncPools()`. This could cause `Sleeves._noUncachedGuard()` to malfunction and revert. In the same way, `Sleeves._withdrawWithProofBatch()` may also revert if a token ID is re-listed in a new pool subsequent to the admin's re-execution of the pool sync function. This could result in users' funds becoming frozen within the contract.

The `Sleeves._sleeveBatch()` function permits depositing any token ID not listed in one of the `PrimeRewards`' contract pools, as `tokenIdsToPoolIds` returns a default pool id of `0`, potentially indicating an incorrect pool. Likewise, `Sleeves._noUncachedGuard()` is vulnerable to being circumvented for tokens associated with a pool ID of `0`, as it accepts any token not registered in pre-synced pools, with `tokenIdsToPoolIds` defaulting to pool ID `0`.

The `claim()` function within the `ERC20Rewarder` contract currently exhibits a security vulnerability. This function allows any user to initiate a claim on behalf of another address. While this could potentially be leveraged for beneficial features, it poses a significant risk in scenarios if a contract interacting with `ERC20Rewarder` does not implement the necessary logic to securely manage such transfer. If exploited, malicious actors could claim ERC20 tokens, potentially freezing the assets in the third-party contract.

The function `adminSetMaxDepositsPerCard()` can update the `maxDepositsPerCard()` to have a value less than the current balance of the contract, which will get the contract into a state of having more sleeve tokens than what is allowed.

In order to validate the proper deployment and initialization of the contracts, it is a good practice to emit events. Also, any important state transitions can be logged, which is beneficial for monitoring the contract, and also tracking eventual bugs or hacks. Below we present a non-exhaustive list of events that could be emitted to improve application management: 1. `ERC20Rewarder.setMerkelRoot()` should emit an event with the updated Merkle root. 1. `Sleeves.adminSyncPools()` should emit an event with the updated pool length. 2. `Sleeves.adminSetUnsleeveBaseFee()` should emit an event with the updated unsleeved base fee. 3. `Sleeves.adminSetRewarder()` should emit an event with the updated Rewarder address.

The `PDTStakingBatch.stake()` function calls `PDTStaking.stake(to, amount)`, which requires the `msg.sender` to have approved the amount to be staked. However, `PDTStakingBatch.stake()` lacks the necessary approval logic, leading to its execution being blocked and the transaction reverted.

It is important to validate inputs, even if they only come from trusted addresses, to avoid human error. Specifically, the following functions arguments should be validated: 1. `Sleeves.constructor()`: 1. `Prime`, `Pdt`, `ParallelAlpha`, `PrimeRewards`, and `Rewarder` should be validated against the zero address. 1. `Sleeves.adminSetRewarder()`: 1. `Rewarder` should be validated against the zero address. 1. `PDTStaking.constructor()`: 1. `PDTStakings` should be validated against the zero address. 1. `ERC20Rewarder.setMerkelRoot()`: 1. `merkleRoot` should be validated to ensure it is not 0x0.

To ensure correct behavior it is advised always to check the return value of function calls if they do return a value. In this regard, consider the following cases: 1. `ERC20Rewarder.claim()` does not check the returned value from the `IERC20.transfer()` call. 2. `Sleeves.adminTransferERC20()` does not check the returned value from the `IERC20.transfer()` call.

The `ERC20Rewarder.claim()` uses `ERC20.transfer()`to send reward to users, meaning that the assets needs to be held by the contract itself, risking either insufficient or excessive rewards being sent, which could affect proper distribution.