PA1D._payoutTokens() won't work for USDT and other inconsistent ERC20 tokens.

[code423n4]

Lines of code

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

Vulnerability details

Impact

Some ERC20 tokens (USDT, BNB, OMG) do not return a boolean on succesful transfer. Checking the returned value of transfer for these tokens will always fail.

Proof of Concept

Usage of ERC20 interface and require statement in PA1D.sol.

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

Recommended Mitigation Steps

Implement a custom function to transfer tokens by checking if the contract exist and making a a low level call using the ERC20 interface selector. E.g.

function _safeTransfer(address token, address to, uint256 value) private {
    require(token.code.length > 0);
    (bool success, bytes memory data) =
    token.call(abi.encodeWithSelector(ERC20.transfer.selector, to, value));
    require(success && (data.length == 0 || abi.decode(data, (bool))));
}

Alternatively, use OpenZeppelin SafeERC20.

[gzeoneth]

Duplicate of #456