code-423n4 / 2022-12-gogopool-findings

1 stars 0 forks source link

Rewards delay release could cause yields steal and loss #863

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2022-12-gogopool/blob/main/contracts/contract/tokens/TokenggAVAX.sol#L88-L109

Vulnerability details

Impact

In the current rewards accounting, vault shares in deposit() and redeem() can not correctly record the spot yields generated by the staked asset. Yields are released over the next rewards cycle. As a result, malicious users can steal yields from innocent users by picking special timing to deposit() and redeem().

Proof of Concept

https://github.com/code-423n4/2022-12-gogopool/blob/main/contracts/contract/tokens/TokenggAVAX.sol#L88-L109

        '   function syncRewards() public {
    uint32 timestamp = block.timestamp.safeCastTo32();

    if (timestamp < rewardsCycleEnd) {
        revert SyncError();
    }

    uint192 lastRewardsAmt_ = lastRewardsAmt;
    uint256 totalReleasedAssets_ = totalReleasedAssets;
    uint256 stakingTotalAssets_ = stakingTotalAssets;

    uint256 nextRewardsAmt = (asset.balanceOf(address(this)) + stakingTotalAssets_) - totalReleasedAssets_ - lastRewardsAmt_;

    // Ensure nextRewardsCycleEnd will be evenly divisible by `rewardsCycleLength`.
    uint32 nextRewardsCycleEnd = ((timestamp + rewardsCycleLength) / rewardsCycleLength) * rewardsCycleLength;

    lastRewardsAmt = nextRewardsAmt.safeCastTo192();
    lastSync = timestamp;
    rewardsCycleEnd = nextRewardsCycleEnd;
    totalReleasedAssets = totalReleasedAssets_ + lastRewardsAmt_;
    emit NewRewardsCycle(nextRewardsCycleEnd, nextRewardsAmt);
}'

Tools Used

Recommended Mitigation Steps

for the lastRewardsAmt_ not released, allow the users to redeem as it is linearly released later. for the accumulated yields, only allow users to redeem the yields received after 1 rewards cycle after the deposit.

c4-judge commented 1 year ago

GalloDaSballo marked the issue as duplicate of #796

c4-judge commented 1 year ago

GalloDaSballo marked the issue as duplicate of #478

GalloDaSballo commented 1 year ago

In contrast to the original dups of 478 these dups are not as accurate, awarding half

c4-judge commented 1 year ago

GalloDaSballo marked the issue as partial-50