Zero token transfer can cause a potential DoS in Distributor.sol

[codehawks-bot]

Zero token transfer can cause a potential DoS in Distributor.sol

Severity

Medium Risk

Relevant GitHub Links

https://github.com/code-423n4/2023-04-caviar-findings/issues/278

Summary

The Distributor.sol contract doesn't check for zero amount while transferring rewards, which can end up blocking the operation.

Vulnerability Details

• In Distributor.sol contract, the function distribute() is used to distribute token to winners according to the percentages: here

  function distribute(address token, address[] memory winners, uint256[] memory percentages, bytes memory data)
      external
  {
      if (msg.sender != FACTORY_ADDRESS) {
          revert Distributor__OnlyFactoryAddressIsAllowed();
      }
      _distribute(token, winners, percentages, data);
  }

• Then function _distribute() is called: here

  116:    function _distribute(address token, address[] memory winners, uint256[] memory percentages, bytes memory data)
  117:        internal
  118:    {
  ---SKIP---
  144:        uint256 winnersLength = winners.length; // cache length
  145:        for (uint256 i; i < winnersLength;) {
  146:            uint256 amount = totalAmount * percentages[i] / BASIS_POINTS;
  147:            erc20.safeTransfer(winners[i], amount);
  148:            unchecked {
  149:                ++i;
  150:            }
  151:        }
  152:
  153:        // send commission fee as well as all the remaining tokens to STADIUM_ADDRESS to avoid dust remaining
  154:        _commissionTransfer(erc20);
  155:        emit Distributed(token, winners, percentages, data);
  156:    }

• At L146-L147,amount after calculated will be transferred to winners[i] but doesn't check for zero amount before transferring.amount will returns 0 if totalAmount * percentages[i] < BASIS_POINTS.
• This is a bit concerning as some ERC20 implementations revert on zero value transfers (see https://github.com/d-xo/weird-erc20#revert-on-zero-value-transfers).
• So the current implementation may cause a denial of service, as a zero amount transfer in this token will block the whole action and revert the transaction.

  Impact

  The function Distributor.sol#distribute() doesn't check for zero amount while transferring rewards, which can end up blocking the operation.

  Tools Used

  Manual review

  Recommendations

  Check for zero amount before executing the transfer. Eg:

  +++ if (amount == 0) revert;

[Nadinnnn]

Escalate

This issue is being identified as a duplicate of Some ERC20 tokens will revert when passing a value of 0. (https://github.com/Cyfrin/2023-08-sparkn/issues/155) .

But it is exactly a duplicate of the issue Potential Distribution Failure Due to Zero Rounded Transfer Amount (https://github.com/Cyfrin/2023-08-sparkn/issues/839) .