getTokenPayout() issues

[code423n4]

Lines of code

https://github.com/code-423n4/2022-10-holograph/blob/main/contracts/enforcer/PA1D.sol#L507

Vulnerability details

Impact

Will not work with some tokens.
Possibility of stealing funds.

Proof of Concept

enforcer/PA1D.sol

function getTokenPayout(address tokenAddress) public {
    _validatePayoutRequestor();
    _payoutToken(tokenAddress);
  }




function _validatePayoutRequestor() private view {
    if (!isOwner()) {
      bool matched;
      address payable[] memory addresses = _getPayoutAddresses();
      address payable sender = payable(msg.sender);
      for (uint256 i = 0; i < addresses.length; i++) {
        if (addresses[i] == sender) {
          matched = true;
          break;
        }
      }
      require(matched, "PA1D: sender not authorized");
    }
  }


function _payoutToken(address tokenAddress) private {
    address payable[] memory addresses = _getPayoutAddresses();
    uint256[] memory bps = _getPayoutBps();
    uint256 length = addresses.length;
    ERC20 erc20 = ERC20(tokenAddress);
    uint256 balance = erc20.balanceOf(address(this));
    require(balance > 10000, "PA1D: Not enough tokens to transfer");
    uint256 sending;
    //uint256 sent;
    for (uint256 i = 0; i < length; i++) {
      sending = ((bps[i] * balance) / 10000);
      require(erc20.transfer(addresses[i], sending), "PA1D: Couldn't transfer token");
    }
  }

_validatePayoutRequestor() performs a simple check, msg.sender should be in payout address list.
_payoutToken loops over payout address list and transfer tokens accoring to their bps.

• _payoutToken() will not work correctly with ERC20 tokens which does not return a value from transfer()

• we could specify arbitrary tokenAddress and do a transfer from any ERC20 token where this contract has funds

• getTokenPayout() is reentrant but I would like to see more test cases which are using this contract

Tools Used

vim

Recommended Mitigation Steps

[gzeoneth]

Duplicate of #456