hats-finance / Fenix--0x9d7765a7ebd5b6322a30797a44a5428531970d3d

0 stars 1 forks source link

checkpointLastIndex and balanceCheckpoints is not updated when amount is withdrawn #51

Open hats-bug-reporter[bot] opened 2 months ago

hats-bug-reporter[bot] commented 2 months ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0x7f942d0a184e0db6749745f4ffbe92ad891a19fddca1a25e56e166cb95e34b30 Severity: medium

Description: Description\ In the SingelTokenVirtualRewarderUpgradeable contract, the withdraw function is responsible for updating the token's balance and total supply. However, there is an issue where the checkpointLastIndex and balanceCheckpoints are not updated when an amount is withdrawn using withdraw() function. This can lead to inconsistencies in the checkpoint system, affecting the accuracy of balance tracking and reward calculations.

Attack Scenario\ Describe how the vulnerability can be exploited.

Attachments

  1. Proof of Concept (PoC) File

    below is the withdraw() function,

    function withdraw(uint256 tokenId_, uint256 amount_) external onlyStrategy {
    TokenInfo storage info = tokensInfo[tokenId_];
    if (info.balance == 0 || amount_ == 0) {
        revert ZeroAmount();
    }
    
    info.balance -= amount_;
    totalSupply -= amount_;
    
    uint256 currentEpoch = _currentEpoch();
    _writeCheckpoints(info, currentEpoch);
    
    emit Withdraw(tokenId_, amount_, currentEpoch);
    }

    as we can see here only the info.balance and Epoch is updated using the _writeCheckpoints() function.

    function _writeCheckpoints(TokenInfo storage info_, uint256 epoch_) internal {
        info_.checkpointLastIndex = VirtualRewarderCheckpoints.writeCheckpoint(
            info_.balanceCheckpoints,
            info_.checkpointLastIndex,
            epoch_,
            info_.balance
        );
    
        totalSupplyCheckpointLastIndex = VirtualRewarderCheckpoints.writeCheckpoint(
            totalSupplyCheckpoints,
            totalSupplyCheckpointLastIndex,
            epoch_,
            totalSupply
        );
    }

    here the _writeCheckPoints() function calls the VirtualRewarderCheckpoints.writeCheckpoint with only updated values of epoch_ and info.balance.

  2. Revised Code File (Optional)

    update both the checkpointLastIndex and balanceCheckpoints before calling the _writeCheckpoints in withdraw()

0xmahdirostami commented 1 month ago

If the withdraw function checkpoints the balance and supply before, then it would overwrite the repetitive amounts / The best way is changing the variables and then checkpointing them