Omniscia Evergon Labs Audit
StakingSkeleton Manual Review Findings
StakingSkeleton Manual Review Findings
SSN-01M: Potential Invalid Restake Re-Entrancy
| Type | Severity | Location |
|---|---|---|
| Logical Fault | ![]() | StakingSkeleton.sol:L585-L590, L596-L601, L604 |
Description:
The StakingSkeleton::fullyUnstake function will transfer rewards towards the user, their corresponding stake, and then will proceed to burn their position.
An issue with this approach is that if the recipient of the reward / stake transfer (i.e. an EIP-1155 transfer) is a smart contract that re-enters the staking system and performs a restake operation, the codebase might ultimately burn an NFT ID with funds associated with it.
Impact:
We consider the vulnerability minor as a user performing the steps outlined in the vulnerability would harm themselves, however, the behaviour might arise from an automated smart contract that is built to automatically restake EIP-1155 assets sent to it.
Example:
584if (amountOfRewardPackets > 0) {585 ITransferRewardFacet(address(this)).transferReward(586 campaignId,587 campaignInfo.rewardAssetHandler,588 msg.sender,589 amountOfRewardPackets590 );591
592 emit RewardsReceived(campaignId, nftId, msg.sender, amountOfRewardPackets);593}594
595if (amountOfPacketsUnstaked > 0) {596 ITransferInputFacet(address(this)).transferInput(597 campaignId,598 campaignInfo.inputAssetKeeper,599 msg.sender,600 amountOfPacketsUnstaked601 );602}603
604IERC721Facet(address(this)).burn(nftId);Recommendation:
We advise the code to burn the NFT prior to transferring the relevant EIP-1155 assets, preventing such a re-entrancy corruption to occur.
Alleviation (b64b659786cf3c84bea52feb3a69f546ba3601f0):
The burn operation has been relocated before the reward and normal packets are disbursed, alleviating this exhibit in full.
SSN-02M: Inexistent Validation of Authorization
| Type | Severity | Location |
|---|---|---|
| Logical Fault | ![]() | StakingSkeleton.sol:L298-L305, L317-L319, L367-L368 |
Description:
The StakingSkeleton::restakeBeneficiary function permits anyone to perform a staking operation of at least one packet to another user without validating any authorization between them.
This permits a user to utilize a single staking packet to arbitrarily increase the timeLockPeriod, thereby potentially locking a victim's significantly large staking position perpetually.
Impact:
The system presently permits all staking positions to be sabotaged by a single packet deposit through restake operations increasing their unlock timestamp to an arbitrarily high number in the future.
Example:
275/**276 * @notice Allows users to increase a beneficiary's position specified by `nftId` within a campaign277 * by allocating additional staked assets defined in the campaign's input packet.278 *279 * @dev This function enables increasing an existing position on behalf of a beneficiary.280 * The staking input packets are transferred from the caller (`msg.sender`), but any rewards are281 * allocated to the beneficiary. The beneficiary must own the NFT representing the position.282 *283 * Restake is not supported when instant staking rewards are featured.284 * The recalculated `unlockTimestamp` (`block.timestamp + timeLockPeriod`) must be greater than or equal to285 * the current position's `unlockTimestamp`.286 *287 * IMPORTANT: Only the beneficiary must meet the eligibility criteria required to increase a position288 * (see `StakersEligibility` and `AccessControl` facets).289 *290 * Emits a {StakingPositionIncreased} and {PositionBalanceUpdate} event.291 * Also emits a {RewardsReceived} event on condition.292 *293 * @param nftId The unique identifier of the NFT associated with the position.294 * @param amountOfPackets The number of input packets to allocate to the existing position, increasing it.295 * @param timeLockPeriod The duration, in seconds, for which the position's staked assets will be locked.296 * @param beneficiary The address that will receive any instant rewards and must own the NFT position.297 */298function restakeBeneficiary(299 uint256 nftId,300 uint256 amountOfPackets,301 uint256 timeLockPeriod,302 address beneficiary303) external onlyExternalDelegateCall {304 _restakeBeneficiary(nftId, amountOfPackets, timeLockPeriod, beneficiary);305}306
307/**308 * @notice Internal version of `restakeBeneficiary()`, handling restaking logic for a specified beneficiary.309 * @dev For more information, see `restakeBeneficiary()`.310 */311function _restakeBeneficiary(312 uint256 nftId,313 uint256 amountOfPackets,314 uint256 timeLockPeriod,315 address beneficiary316) internal {317 if (beneficiary != IERC721Facet(address(this)).ownerOf(nftId)) {318 revert StakingPositionNotOwned(nftId, beneficiary);319 }320
321 GeneralStorage.NftInfo storage nftInfo = GeneralStorage.layout().nftInfo[nftId];322 uint256 campaignId = nftInfo.campaignId;323
324 IAmountsFacet(address(this)).checkInputPackets(campaignId, nftId, amountOfPackets);325 IStakersEligibilityFacet(address(this)).checkStakersEligibility(campaignId, beneficiary);326 uint256 stakeActiveStartingTimestamp = ICampaignTimesFacet(address(this)).checkCampaignTimesOnStake(campaignId);327 ILockVariationsFacet(address(this)).checkTimeLock(campaignId, timeLockPeriod);328
329 uint256 newUnlockTimestamp = stakeActiveStartingTimestamp + timeLockPeriod;330
331 if (newUnlockTimestamp < nftInfo.unlockTimestamp) {332 // Cannot have earlier unlock timestamp than already existing one333 revert InvalidUnlockTimestampAtRestake(nftId, nftInfo.unlockTimestamp, newUnlockTimestamp);334 }335
336 GeneralStorage.Campaign storage campaignInfo = GeneralStorage.layout().campaignsInfo[campaignId];337
338 ITransferInputFacet(address(this)).transferInput(339 campaignId,340 msg.sender,341 campaignInfo.inputAssetKeeper,342 amountOfPackets343 );344
345 uint256 totalPackets = nftInfo.packetsStaked + amountOfPackets;346
347 uint256 getRestakeRewardPackets = IRewardsDistributionFacet(address(this)).getRestakeReward(campaignId, nftId);348 uint256 virtualTotalPackets = IVirtualAmountMultiplierFacet(address(this)).applyVirtualAmountMultiplier(349 campaignId,350 nftId,351 totalPackets352 );353
354 emit StakingPositionIncreased(355 campaignId,356 nftId,357 beneficiary,358 msg.sender,359 amountOfPackets,360 stakeActiveStartingTimestamp,361 nftInfo.unlockTimestamp,362 newUnlockTimestamp363 );364
365 // The `nftInfo` storage is updated after calling `getRestakeReward()`,366 // as this function relies on the current status of the position (prior to this restake)367 nftInfo.unlockTimestamp = newUnlockTimestamp;368 nftInfo.startingTimestamp = stakeActiveStartingTimestamp;369
370 emit PositionBalanceUpdate(371 campaignId,372 nftId,373 beneficiary,374 nftInfo.packetsStaked,375 nftInfo.virtualPacketsStaked,376 totalPackets,377 virtualTotalPackets378 );379
380 campaignInfo.totalPacketsStaked += amountOfPackets;381 campaignInfo.virtualTotalPacketsStaked += (virtualTotalPackets - nftInfo.virtualPacketsStaked);382 nftInfo.packetsStaked = totalPackets;383 nftInfo.virtualPacketsStaked = virtualTotalPackets;384
385 _applyRestakeAndTransferRewards(386 campaignId,387 nftId,388 getRestakeRewardPackets,389 campaignInfo.rewardAssetHandler,390 beneficiary391 );392}Recommendation:
We advise the system to either impose some form of authorization between the caller and the beneficiary, or to disallow the position's unlockTimestamp from increasing by calculating the remaining time until the stake unlocks and utilizing that as the timeLockPeriod.
We consider either of the two approaches adequate in alleviating this exhibit.
Alleviation (b64b659786cf3c84bea52feb3a69f546ba3601f0):
The code was updated to prevent the increase of an active NFT's unlock timestamp, ensuring that the code prevents the unlock timestamp of an NFT from being increased by restake operations.

