Omniscia Evergon Labs Audit
StakingSkeletonNID Manual Review Findings
StakingSkeletonNID Manual Review Findings
SSI-01M: Potential Invalid Restake Re-Entrancy
| Type | Severity | Location |
|---|---|---|
| Logical Fault | ![]() | StakingSkeletonNID.sol:L618-L627, L629-L636, L638 |
Description:
The StakingSkeletonNID::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:
618if (amountOfRewardPackets > 0) {619 ITransferRewardFacet(address(this)).transferReward(620 campaignId,621 campaignInfo.rewardAssetHandler,622 msg.sender,623 amountOfRewardPackets624 );625
626 emit RewardsReceived(campaignId, nftId, msg.sender, amountOfRewardPackets);627}628
629if (amountOfPacketsUnstaked > 0) {630 ITransferInputFacet(address(this)).transferInput(631 campaignId,632 campaignInfo.inputAssetKeeper,633 msg.sender,634 amountOfPacketsUnstaked635 );636}637
638IERC721Facet(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.
SSI-02M: Inexistent Validation of Authorization
| Type | Severity | Location |
|---|---|---|
| Logical Fault | ![]() | StakingSkeletonNID.sol:L332-L339, L351-L353 |
Description:
The StakingSkeletonNID::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:
310/**311 * @notice Allows users to increase a beneficiary's position specified by `nftId` within a campaign312 * by allocating additional staked assets defined in the campaign's input packet.313 *314 * @dev This function enables increasing an existing position on behalf of a beneficiary.315 * The staking input packets are transferred from the caller (`msg.sender`), but any rewards are316 * allocated to the beneficiary. The beneficiary must own the NFT representing the position.317 *318 * Restake is not supported when instant staking rewards are featured.319 * The recalculated `unlockTimestamp` (`block.timestamp + timeLockPeriod`) must be greater than or equal to320 * the current position's `unlockTimestamp`.321 *322 * Users are gated by `requireTxDataAuth` modifier (see NID `TxAuthDataVerifierFacet`).323 *324 * Emits a {StakingPositionIncreased} and {PositionBalanceUpdate} event.325 * Also emits a {RewardsReceived} event on condition.326 *327 * @param nftId The unique identifier of the NFT associated with the position.328 * @param amountOfPackets The number of input packets to allocate to the existing position, increasing it.329 * @param timeLockPeriod The duration, in seconds, for which the position's staked assets will be locked.330 * @param beneficiary The address that will receive any instant rewards and must own the NFT position.331 */332function restakeBeneficiary(333 uint256 nftId,334 uint256 amountOfPackets,335 uint256 timeLockPeriod,336 address beneficiary337) external requireTxDataAuth onlyExternalDelegateCall {338 _restakeBeneficiary(nftId, amountOfPackets, timeLockPeriod, beneficiary);339}340
341/**342 * @notice Internal version of `restakeBeneficiary()`, handling restaking logic for a specified beneficiary.343 * @dev For more information, see `restakeBeneficiary()`.344 */345function _restakeBeneficiary(346 uint256 nftId,347 uint256 amountOfPackets,348 uint256 timeLockPeriod,349 address beneficiary350) internal {351 if (beneficiary != IERC721Facet(address(this)).ownerOf(nftId)) {352 revert StakingPositionNotOwned(nftId, beneficiary);353 }354
355 GeneralStorage.NftInfo storage nftInfo = GeneralStorage.layout().nftInfo[nftId];356 uint256 campaignId = nftInfo.campaignId;357
358 IAmountsFacet(address(this)).checkInputPackets(campaignId, nftId, amountOfPackets);359 uint256 stakeActiveStartingTimestamp = ICampaignTimesFacet(address(this)).checkCampaignTimesOnStake(campaignId);360 ILockVariationsFacet(address(this)).checkTimeLock(campaignId, timeLockPeriod);361
362 uint256 newUnlockTimestamp = stakeActiveStartingTimestamp + timeLockPeriod;363
364 if (newUnlockTimestamp < nftInfo.unlockTimestamp) {365 // Cannot have earlier unlock timestamp than already existing one366 revert InvalidUnlockTimestampAtRestake(nftId, nftInfo.unlockTimestamp, newUnlockTimestamp);367 }368
369 GeneralStorage.Campaign storage campaignInfo = GeneralStorage.layout().campaignsInfo[campaignId];370
371 ITransferInputFacet(address(this)).transferInput(372 campaignId,373 msg.sender,374 campaignInfo.inputAssetKeeper,375 amountOfPackets376 );377
378 uint256 totalPackets = nftInfo.packetsStaked + amountOfPackets;379
380 uint256 getRestakeRewardPackets = IRewardsDistributionFacet(address(this)).getRestakeReward(campaignId, nftId);381 uint256 virtualTotalPackets = IVirtualAmountMultiplierFacet(address(this)).applyVirtualAmountMultiplier(382 campaignId,383 nftId,384 totalPackets385 );386
387 emit StakingPositionIncreased(388 campaignId,389 nftId,390 beneficiary,391 msg.sender,392 amountOfPackets,393 stakeActiveStartingTimestamp,394 nftInfo.unlockTimestamp,395 newUnlockTimestamp396 );397
398 // The `nftInfo` storage is updated after calling `getRestakeReward()`,399 // as this function relies on the current status of the position (prior to this restake)400 nftInfo.unlockTimestamp = newUnlockTimestamp;401 nftInfo.startingTimestamp = stakeActiveStartingTimestamp;402
403 emit PositionBalanceUpdate(404 campaignId,405 nftId,406 beneficiary,407 nftInfo.packetsStaked,408 nftInfo.virtualPacketsStaked,409 totalPackets,410 virtualTotalPackets411 );412
413 campaignInfo.totalPacketsStaked += amountOfPackets;414 campaignInfo.virtualTotalPacketsStaked += (virtualTotalPackets - nftInfo.virtualPacketsStaked);415 nftInfo.packetsStaked = totalPackets;416 nftInfo.virtualPacketsStaked = virtualTotalPackets;417
418 _applyRestakeAndTransferRewards(419 campaignId,420 nftId,421 getRestakeRewardPackets,422 campaignInfo.rewardAssetHandler,423 beneficiary424 );425}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.

