sherlock-audit / 2024-05-tokensoft-distributor-contracts-update-judging

3 stars 2 forks source link

Salem - Voting Power Inflation via Token Transfer and Reconciliation #16

Closed sherlock-admin2 closed 4 months ago

sherlock-admin2 commented 4 months ago

Salem

high

Voting Power Inflation via Token Transfer and Reconciliation

Summary

When a user (beneficiary) transfers all their votes to another user, their balanceOf becomes 0. Upon calling _reconcileVotingPower, the function may mint new tokens based on thenewVotes calculation, leading to potential voting power inflation. This can happen because the function does not account for the actual intent of the vote transfer, treating it as a need to mint new tokens to balance the voting power.

Vulnerability Detail

When a user (beneficiary) transfers all their votes to another user, their balanceOf becomes 0. Upon calling _reconcileVotingPower, the function may mint new tokens based on the newVotes calculation, leading to potential voting power inflation. This can happen because the function does not account for the actual intent of the vote transfer, treating it as a need to mint new tokens to balance the voting power.

Impact

Code Snippet

code

Tool used

Manual Review


function _reconcileVotingPower(address beneficiary) private {
    // current potential voting power
    uint256 currentVotes = balanceOf(beneficiary);
    // correct voting power after initialization, claim, or adjustment
    DistributionRecord memory record = records[beneficiary];
    uint256 newVotes = record.claimed >= record.total ? 0 : tokensToVotes(record.total - record.claimed);

    if (currentVotes > newVotes) {
        // reduce voting power through ERC20Votes extension
        _burn(beneficiary, currentVotes - newVotes);
    } else if (currentVotes < newVotes) {
        // increase voting power through ERC20Votes extension
        _mint(beneficiary, newVotes - currentVotes);
    }
}

Recommendation

Implement checks to ensure that minting only occurs when genuinely required and in appropriate contexts. Also, ensure that transferring votes does not result in improper minting.


function _reconcileVotingPower(address beneficiary) private {
    // Retrieve current token balance of the beneficiary
    uint256 currentVotes = balanceOf(beneficiary);

    // Retrieve the distribution record for the beneficiary
    DistributionRecord memory record = records[beneficiary];

    // Calculate the new correct voting power based on unclaimed tokens
    uint256 newVotes = record.claimed >= record.total ? 0 : tokensToVotes(record.total - record.claimed);

    // Ensure the calculations are safe and within expected bounds
    require(newVotes <= type(uint256).max, "newVotes calculation overflow");

    // Check if the current voting power needs to be adjusted
    if (currentVotes > newVotes) {
        uint256 amountToBurn = currentVotes - newVotes;

        // Verify that the beneficiary has enough tokens to burn
        require(balanceOf(beneficiary) >= amountToBurn, "Insufficient balance to burn");

        // Reduce voting power through ERC20Votes extension by burning tokens
        _burn(beneficiary, amountToBurn);
    } else if (currentVotes < newVotes && newVotes > 0) {
        uint256 amountToMint = newVotes - currentVotes;

        // Increase voting power through ERC20Votes extension by minting tokens
        _mint(beneficiary, amountToMint);
    }

    // Emit an event to record the voting power reconciliation
    emit VotingPowerReconciled(beneficiary, newVotes);
}