Project: PossumLabsCrypto/Core
Commit: ae14254de919ba19809df09bf5a7e4d3070dc625
Start Date: 2024-07-15
Scope:
src/PossumCore.sol(nSLOC: 184, coverage: 100% lines / 98.52% statements / 92.11% branches / 100% functions)
Overview
- [L-01] The guardian is allowed to whitelist the
PossumCorecontract - [L-02] Unused Core Fragments are forfeited on unstaking
- [I-01] Inconsistent UX concerning the clamping of amounts
- [I-02] User protection on zero
PSMbalance can be circumvented by malicious actor - [I-03] Forfeiting of Core Fragments on unstaking can be circumvented due to rounding
The guardian is allowed to whitelist address(this), i.e. the PossumCore contract itself. In this case, core stakers could increase their claimable reservedRewards by distributing to the PossumCore contract without changing its PSM balance. Therefore, no meaningful distribution is made while the total value of rewards, that can be extracted, potentially doubles since distributed PSM stays within the contract.
Add an additional check for address(this) to the updateWhitelist method:
function updateWhitelist(address _destination, bool _listed) external onlyGuardian {
if (_destination == address(0) || _destination == address(this)) {
revert InvalidAddress();
}
...
}Status:
✅ Resolved in commit 338a5e49df17a1c4842dbd0a21a7468bf56795fc.
In the unstakeAndClaim method, the user's storedCoreFragments, which are unused Core Fragments intended for distribution, are forfeited in proportion to the unstaking amount and eventually reach zero on full unstaking.
It is advised to consider an alternative implementation which allows user's to keep and further distribute their unused Core Fragments after unstaking. However, no further interest should be accrued based on the rewards from distributions after unstaking due to the absent base stake.
Status:
🆗 Acknowledged.
In the unstakeAndClaim method, any amount > balance is silently clamped to balance. However, in the distributeCoreFragments method, any amount > userFragments leads to an InvalidAmount error.
It is recommended to always revert on an exceeding amount, but to explicitly clamp the amount to balance/userFragments when type(uint256).max is specified (for better UX).
Status:
✅ Resolved in commits 7cf67e1f2fcae21a1cb6d920f6ed4b996e618724 & 0eded7695af347f387908d7d706de4fc15ecefed.
The stake mehtod employs a if (availablePSM == 0) revert InsufficientRewards() check to prevent users from staking when the PossumCore contract is out of PSM for rewards and distribution. Similary, the distributeCoreFragments method employs the same check to prevent users from executing an empty distribution in such a case.
However, a malicious actor could donate as little as 1 wei of PSM to the PossumCore contract to circumvent this check and therefore remove the present zero balance protection for honest users.
Status:
🆗 Acknowledged.
In the unstakeAndClaim method, the user's storedCoreFragments, which are unused Core Fragments intended for distribution, are forfeited in proportion to the unstaking amount and eventually reach zero on full unstaking.
However, the current computation of fragments to forefeit affectedFragments = (fragments * amount) / balance allows to unstake small amounts while the affectedFragments are still zero due to rounding.
Add a check that prevents unstaking without reducing the user's Core Fragments:
if (affectedFragments == 0 && fragments > 0) revert InvalidAmount();Status:
🆗 Acknowledged.