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

2 stars 1 forks source link

BiasedMerc - AdvancedDistributor::adjust() only decreases token balance, but doesn't increase it when there is a claim amount increase #30

Closed sherlock-admin4 closed 4 weeks ago

sherlock-admin4 commented 1 month ago

BiasedMerc

medium

AdvancedDistributor::adjust() only decreases token balance, but doesn't increase it when there is a claim amount increase

Summary

AdvancedDistributor::adjust() allows to increase/decrease the amount that is claimable by a user. If the amount is decreased then the excess tokens are transfered to the owner however if the amount is increased no tokens are transfered. Meaning that there will not be enough balance to honour all claims.

Vulnerability Detail

AdvancedDistributor::adjust()

  function adjust(address beneficiary, int256 amount) external onlyOwner {
    DistributionRecord memory distributionRecord = records[beneficiary];
    require(distributionRecord.initialized, 'must initialize before adjusting');

    uint256 diff = uint256(amount > 0 ? amount : -amount);
    require(diff < type(uint120).max, 'adjustment > max uint120');

    if (amount < 0) {
      // decreasing claimable tokens
      require(total >= diff, 'decrease greater than distributor total');
      require(distributionRecord.total >= diff, 'decrease greater than distributionRecord total');
      total -= diff;
      records[beneficiary].total -= uint120(diff);
      token.safeTransfer(owner(), diff);
    } else {
      // increasing claimable tokens
      total += diff;
      records[beneficiary].total += uint120(diff);
    }
    _reconcileVotingPower(beneficiary);
    emit Adjust(beneficiary, amount);
  }

When the owner adjusts the claimable amount by a user, if they are reducing the claimable amount then those tokens are transfered to the owner as they are no longer needed within the contract. However if the owner increases the claimable amount, there is no transfer to ensure that those tokens have been sent to the contract, meaning there will not be enough funds in the contract to allow the new increased claim.

Impact

If the claimable amount is increased through AdvancedDistributor::adjust() then there will not be enough tokens in the contract to allow all claims, which will lead to all users being unable to complete their claims, causing a loss of funds.

Code Snippet

AdvancedDistributor::adjust()

Tool used

Manual Review

Recommendation

Add a transferFrom to AdvancedDistributor::adjust() that transfers diff amount of tokens to the contract, to ensure that there will be enough tokens to allow full user claims.

Duplicate of #43